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 note as annotationType #971

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Add note as annotationType #971

wants to merge 8 commits into from

Conversation

ekemeyer
Copy link
Contributor

@ekemeyer ekemeyer commented Mar 3, 2025

PR to add "Note" as a valid annotationType specifically for PBCore Zipped ingest. This is to allow 68K minnesota records to be ingested that had multiple invalid annotationTypes that we have merged into a single "Note" annotationType

@ekemeyer ekemeyer marked this pull request as draft March 3, 2025 16:28
@ekemeyer ekemeyer requested a review from afred March 3, 2025 17:08
@ekemeyer ekemeyer marked this pull request as ready for review March 3, 2025 17:08
Copy link
Contributor

@afred afred left a comment

Choose a reason for hiding this comment

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

In addition to the indentation fixes... are we sure we want to add this type?

To add an annotation of type "note" seems a little redundant (annotations are notes), and also like it may be leaning into a well known anti-pattern of putting specific things into a generic notes field. This has been an issue with every database I've ever worked with -- when the schema falls short, the notes fields get overused.

There is also the annotations without any value for annotationType, which I think was what we were using for adding a generic "note" via pbcoreAnnotation. So if we now have a pbcoreAnnotation with annotationType="note", we may actually be introducing a 2nd place for putting generic notes.


def note
note ||= find_annotation_attribute("note")
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Unindent end by one space to match it's opening def (same for ams1_legacy_metadata above, and any other place you might see where they are unaligned).


def note
note ||= find_annotation_attribute("note")
end
Copy link
Contributor

Choose a reason for hiding this comment

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

match indentation of def and end

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