-
Notifications
You must be signed in to change notification settings - Fork 0
Use 109 normalized record symbols #263
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
Conversation
TIMDEX title to sym
Pull Request Test Coverage Report for Build 19079973874Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gnarly as expected! I did some smoke testing in GeoData and USE against their corresponding staging apps. I'm seeing all the fields I expect to see, and as you noted, you fixed an issue that was preventing metadata in geo_data_info from rendering.
It does make me feel a bit odd that we now use symbols as keys for top-level fields, but continue to use strings as keys for subfields. I think I'm okay with it, though, since it appears to be consistent.
Great work wrangling this! I'm sure it was awful, but it's an excellent refactor.
Yeah, I should have noted that I felt that way too. I'm not opposed to another round of refactor at some point but wanted to get this messy bit done before we decide if we want to take it further. |
Note: I have kept quite a few commits as I realized it might be easier to look through each data element rather than the full change set. I didn't start doing that until I had finished Primo though, but those were simpler so hopefully it'll be fine.
Please check both USE and GDT versions in a running application (PR build or local) and compare to staging tiers to ensure everything works as expected. I think I caught everything and inadvertently fixed a but in GeoData prod while making these changes.
https://mitlibraries.atlassian.net/browse/USE-109
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing