-
Notifications
You must be signed in to change notification settings - Fork 4
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
[DUOS-1958][risk=no] NIH And AnVIL Use Section #1800
Conversation
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.
One syntax fix, then 👍🏽 Optionally, look into refactoring the constants for later optimization. Not required for this PR.
const I_DID = 'I Did'; | ||
const I_WILL = 'I Will'; | ||
const NO = 'No'; | ||
|
||
const nihAnvilUseLabels = { | ||
i_did: I_DID, | ||
i_will: I_WILL, | ||
no: NO | ||
}; |
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.
One thing that I think would be helpful is to extract all of these kinds of constants to a utility function that describes ALL of the things in a dataset registration. Later on, when the UI has access to the json schema in consent, then that utility function can be updated more easily to take advantage of existing code and reduce the duplication.
Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>
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.
Changes and the new NIH and AnVIL use section on the Dataset Registration page looks good to me.
Addresses
https://broadworkbench.atlassian.net/browse/DUOS-1958
The
FormData
components had matured quite a bit, so I recreated Shae's changes in a new branch to make the work simpler.Have you read Terra's Contributing Guide lately? If not, do that first.