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

DM-48178: Add documentation for DRP schemas. #299

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Conversation

TallJimbo
Copy link
Member

Checklist

When making changes to YAML files in the schemas directory:

  • If applicable, incremented the schema version number, following the guidelines in the contribution guide
  • Referred to the documentation on specific schemas for additional versioning information, change constraints, or tasks that may need to be performed, based on which schema is being updated

@JeremyMcCormick JeremyMcCormick self-requested a review January 10, 2025 21:35
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @TallJimbo !

This should be useful information, especially for repository contributors.

I have made a few relatively minor inlined suggestions:

  • Added links for referenced GitHub packages and files so that they are more easily accessible.
  • Changed some repository and file links to absolute style, as github.com should be unneeded.
  • Made a few minor changes to the wording and grammar.

I have left this in "Request changes" state until I am able to discuss with @gpdf, in case he would like to see more major changes in the form of additional information, etc.

My only suggestion for additional text might be including some information on who should be considered responsible for reviewing PRs on the DRP schemas. You had mentioned to me that the reviewers of the pipeline updates should also be reviewing the corresponding sdm_schemas updates. Would this information be worth stating explicitly here, especially for the benefit of new Rubin staff members working on the pipelines who may not be completely familiar with the procedure?

docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
docs/DRP.md Outdated Show resolved Hide resolved
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-48178 branch 5 times, most recently from f74d5df to 265d527 Compare January 30, 2025 20:20
@JeremyMcCormick
Copy link
Collaborator

I have made some minor edits to the original draft, mostly just adding links.

@TallJimbo This looks good to me now.

Please feel free to make minor corrections/additions before merging.

@TallJimbo TallJimbo merged commit 9aba8a0 into main Jan 30, 2025
10 checks passed
@TallJimbo TallJimbo deleted the tickets/DM-48178 branch January 30, 2025 23:11
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