Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add AppendUIDData and CopyUIDData classes #400

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented Feb 6, 2025

It isn't possible to convert UIDPlusData to use SequenceSet internally, while still maintaining full backwards compatibility. So two new classes were added (AppendUIDData and CopyUIDData). The intention is to remove UIDPlusData in v0.6.0.

  • Both AppendUIDData and CopyUIDData:
    • uid-set attributes are SequenceSet objects (not arrays of integers)
    • new validations have been added to the initializers
    • data.size to get the uid-set size
  • Only CopyUIDData:
    • uid_mapping => source_uid_to_assigned_uid_hash
      (This behaves identically to the existing UIDPlusData#uid_mapping.)
    • copyuid.assigned_uid_for(source_uid) => assigned_uid
      copyuid[source_uid] => assigned_uid
    • copyuid.source_uid_for(assigned_uid) => source_uid
    • copyuid.each_uid_pair (aliases: each_pair, each)

Although they are not perfectly compatible, upgrading from UIDPlusData to CopyUIDData and AppendUIDData should be relatively straightforward.

This PR only adds the classes, but they are unused. For backward compatibility, a parser config option is needed in order to replace UIDPlusData with them. That was split into a separate PR (mostly just to simplify the release notes generation):

@nevans nevans changed the title ✨ Add AppendUIDData/CopyUIDData, 🔧 Add parser config for APPENDUID/COPYUID, 🗑️ Deprecate UIDPlusData` ✨ Add AppendUIDData/CopyUIDData, 🔧 Add parser config for APPENDUID/COPYUID, 🗑️ Deprecate UIDPlusData Feb 6, 2025
@nevans nevans changed the title ✨ Add AppendUIDData/CopyUIDData, 🔧 Add parser config for APPENDUID/COPYUID, 🗑️ Deprecate UIDPlusData ✨ Add AppendUIDData/CopyUIDData, 🔧 Add parser config for APPENDUID/COPYUID, 🗑️ Deprecate UIDPlusData Feb 6, 2025
@nevans nevans force-pushed the add-appenduid-copyuid-classes branch from ec7fb2c to bcb261d Compare February 6, 2025 16:55
@nevans nevans changed the title ✨ Add AppendUIDData/CopyUIDData, 🔧 Add parser config for APPENDUID/COPYUID, 🗑️ Deprecate UIDPlusData ✨ Add AppendUIDData and CopyUIDData classes Feb 6, 2025
@nevans nevans requested a review from shugo February 6, 2025 17:18
@nevans nevans merged commit 8f41dea into master Feb 6, 2025
37 checks passed
@nevans nevans deleted the add-appenduid-copyuid-classes branch February 6, 2025 17:26
@nevans
Copy link
Collaborator Author

nevans commented Feb 6, 2025

@hoffi Since you submitted the original code, I assume this may affect you. Any thoughts? (See also: #401)

@hoffi
Copy link
Contributor

hoffi commented Feb 7, 2025

Looks fine to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants