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

Removing many rendition properties from the ninjs 3.0 draft #195

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

bquinn
Copy link
Contributor

@bquinn bquinn commented Sep 15, 2024

For now we are focusing on properties that were already in ninjs 2.1 or are required for DPP LPX. Fixes #189 and #194.

are focusing on properties that were already in ninjs 2.1 or are
required for DPP LPX.
@bquinn bquinn changed the title Removing many rendition properties from the draft Removing many rendition properties from the ninjs 3.0 draft Sep 15, 2024
@bquinn bquinn requested a review from iyoung September 15, 2024 11:16
@bquinn bquinn requested a review from himslm01 September 16, 2024 08:53
Copy link
Contributor

@iyoung iyoung left a comment

Choose a reason for hiding this comment

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

This looks good to me @bquinn , aligning with the DPP LPX minimum required changes to renditions. A number of suggested non SRT fields have also been removed in this change, I just want to check that's intentional presumably for further discussion before they're included?

as discussed with Brendan adding back in poi
@iyoung
Copy link
Contributor

iyoung commented Sep 30, 2024

I've added POI back in as discussed @bquinn if there's nothing else on this I think we can get this merged to tidy up the renditions. I added a comment on #164 (comment)

@bquinn
Copy link
Contributor Author

bquinn commented Sep 30, 2024

LGTM. All tests pass.

@bquinn bquinn merged commit a02885e into events-planning Sep 30, 2024
1 check passed
@bquinn bquinn deleted the renditions-tidyup branch September 30, 2024 16:16
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