fix: Migrate buried_on/buried_at to burial events#651
Open
noahbjohnson wants to merge 5 commits intomainfrom
Open
fix: Migrate buried_on/buried_at to burial events#651noahbjohnson wants to merge 5 commits intomainfrom
noahbjohnson wants to merge 5 commits intomainfrom
Conversation
glx migrate only converted born_on/born_at/died_on/died_at to birth/death events, leaving buried_at as an unconverted person property. Add the same treatment for buried_on/buried_at: create burial events, retarget assertions, remove deprecated properties, reject them in validation, and skip them in GEDCOM export. Fixes #645
Deploying genealogix with
|
| Latest commit: |
441eadd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0a04e4b2.genealogix.pages.dev |
| Branch Preview URL: | https://fix-buried-at-migration.genealogix.pages.dev |
golangci-lint v2.11 removed the `sort-results` key from the `output` section (the `sort-order` key already implies sorting). This broke CI config verification.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the GLX tooling to fully deprecate buried_on/buried_at by migrating them into burial events (matching existing birth/death behavior), validating against their use, and preventing their export as generic GEDCOM person tags.
Changes:
- Extend
glx migrateto convertburied_on/buried_atinto burial events, merge into existing burial events, and retarget assertions. - Add validation errors for
buried_on/buried_atwith guidance to use burial events instead. - Skip
buried_on/buried_atduring GEDCOM person-property export; update docs/changelog and add/adjust tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go-glx/validation.go | Treat buried_on/buried_at as removed properties with migration guidance. |
| go-glx/validation_test.go | Add tests asserting validation errors for buried_on/buried_at. |
| go-glx/gedcom_export_person.go | Add burial deprecated properties to the person-property skip list. |
| go-glx/gedcom_export_test.go | Extend export test coverage to ensure burial deprecated props are not exported; includes minor formatting adjustments. |
| go-glx/constants.go | Add DeprecatedPropertyBuriedOn / DeprecatedPropertyBuriedAt constants and update related comments/formatting. |
| glx/migrate_runner.go | Migrate buried_on/buried_at into burial events and retarget relevant assertions; include in deprecated vocab cleanup. |
| glx/migrate_runner_test.go | Add migration tests covering burial creation, merge behavior, assertion retargeting, and combined vital-event migration. |
| glx/cmd_migrate.go | Update glx migrate help text to include burial properties/events. |
| glx/README.md | Document burial migration behavior in CLI docs. |
| docs/guides/hands-on-cli-guide.md | Update hands-on guide to include burial migration behavior. |
| CHANGELOG.md | Add changelog entry describing the burial migration fix (issue #645). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…erties The function now handles burial in addition to birth/death. Update the name and doc comment to reflect the full scope.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The sort-results removal (previous commit) unblocked config validation, which meant the linter now actually runs — surfacing 784 pre-existing issues. Add only-new-issues: true so PRs only fail on issues introduced by the change.
Extract migratePropertyPair and migrateOrphanedAssertions helpers from migrateVitalEventProperties to bring cognitive complexity under the gocognit threshold. Add eventFieldDate/eventFieldPlace constants to satisfy goconst.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
glx migrateconvertedborn_on/born_at/died_on/died_atto birth/death events but did not handleburied_at(orburied_on). After migration,buried_atsurvived as an unconverted person property, causingglx summaryto show no burial event while the place lingered as a raw property.This PR gives
buried_on/buried_atthe same treatment as the other deprecated properties:buried_on/buried_atwith "use burial events instead"DeprecatedPropertyBuriedOnandDeprecatedPropertyBuriedAtRelated issues
Fixes #645
Testing
make testpasses,golangci-lint run --new-from-revreports 0 issuesBreaking changes
None. Archives with
buried_on/buried_atthat previously passed validation will now receive errors guiding users to runglx migrate.