-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
📝 Add clarifying comments in default pipeline config #1797
base: develop
Are you sure you want to change the base?
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.
Looks great! I wonder if we want to merge now or wait until we redo the naming to have regressor names in the files? @sgiavasis @shnizzedy ?
Good question. Since this PR is marked as a draft I thought it wasn't ready to merge yet, but I think as it stands
I don't see any harm in clarifying while necessary and updating/removing once we update the |
Co-authored-by: Jon Clucas <[email protected]>
@shnizzedy, @gkiar I'm fine with merging this now if you want. But I planned to add more notes after going through the default config in more detail. Lmk what you want to do. |
Up to you! I think we should at least wait until #1825 and/or #1831 get merged in so we keep the regressor-fork examples in sync with what C-PAC actually does, but I'm fine either way if you want to go ahead and merge soon or wait until closer to release time if you think you'll get through more of the default config by then |
Description
This PR adds a few clarifying comments to the default
pipeline_config_default.yml
to help users understand the different config options.Checklist
Update index.md
).develop
branch of the repository.Developer Certificate of Origin
Developer Certificate of Origin