-
Notifications
You must be signed in to change notification settings - Fork 11
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
dandi-schema readme edit #152
Conversation
Codecov ReportBase: 96.61% // Head: 96.61% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
=======================================
Coverage 96.61% 96.61%
=======================================
Files 16 16
Lines 1623 1623
=======================================
Hits 1568 1568
Misses 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
Text looks good. My suggestions are mostly to do with formatting/style.
## Description | ||
|
||
Every `Dandiset` and associated asset has a metadata object that can be retrieved using | ||
the DANDI API. This library helps create and validate DANDI schema-compliant metadata for `Dandisets` |
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.
the DANDI API. This library helps create and validate DANDI schema-compliant metadata for `Dandisets` | |
the DANDI API. This library helps create and validate DANDI schema-compliant metadata for Dandisets |
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.
we formatted "Dandisets" consistently that way throughout the handbook, using the code element, so I was following suit
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.
Then I agree that, for consistency, it should be formatted as a code element.
But on a broader level, I don't think that is the right choice. Can you point me to the style policy/discussion where this decision was made?
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.
there was never any discussion or decision made, it had been consistently styled that way before I came on board so I followed that set standard; in hindsight I agree it's not a great idea to use the code element as way to emphasize a term that isn't code! we have an entry in the stylesheet for spelling the term with an initial cap, we can revisit and decide whether to also universally remove the code formatting
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.
Let's make the decision now to remove the code element from the term "Dandiset". Can you update the stylesheet, @melster1010?
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.
@waxlamp sure - I'll have to update the stylesheet, remove the special formatting from "Dandiset" in the Handbook files, as well as this readme (as separate PR)
Co-authored-by: Roni Choudhury <2903332+waxlamp@users.noreply.github.com>
Co-authored-by: Roni Choudhury <2903332+waxlamp@users.noreply.github.com>
Co-authored-by: Roni Choudhury <2903332+waxlamp@users.noreply.github.com>
Edit readme for stylesheet compliance/grammar/clarity/consistency.