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

Fix up polymorphism for core model types #5

Merged
merged 4 commits into from
Feb 7, 2025
Merged

Conversation

mdholloway
Copy link
Owner

@mdholloway mdholloway commented Jan 25, 2025

Previously the manuscript, holding, and record classes weren't doing much. They were added with an eye toward polymorphism, but the ExportRepresenter was converting all item hashes unconditionally to a generic DsItem object with methods applicable to all three.

This change gets ExportRepresenter working as intended so that Holding, Manuscript, and Record objects are created by ExportRepresenter where appropriate. DsItem is removed as no longer needed, and all unused methods of the core model types are removed as well.

The spec helper export hash is also updated to support these changes.

Addresses comment on #3

@mdholloway
Copy link
Owner Author

I haven't yet tested the conversion script with these changes. I think it'll need a tweak or two. I'll try to get to that later today.

Copy link
Collaborator

@emeryr-upenn emeryr-upenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. These are suggestions for minor changes. Only one, the recommendation for DsMeta is for a potential bug.

@mdholloway mdholloway assigned emeryr-upenn and unassigned mdholloway Feb 2, 2025
@mdholloway
Copy link
Owner Author

Thanks for the suggestions! I've applied them all.

mdholloway and others added 3 commits February 2, 2025 18:44
Previously the manuscript, holding, and record classes weren't doing much.
They were added with an eye toward polymorphism, but the ExportRepresenter
was converting all item hashes unconditionally to a generic DsItem object
with methods applicable to all three.

This change gets ExportRepresenter working as intended so that Holding,
Manuscript, and Record objects are created by ExportRepresenter where
appropriate. DsItem is removed as no longer needed, and all unused methods
of the core model types are removed as well.

The spec helper export hash is also updated to accommodate these changes.

Addresses comment on #3
@mdholloway mdholloway merged commit 694c65d into main Feb 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants