-
Notifications
You must be signed in to change notification settings - Fork 3
NH-116007: update contributing and config doc #221
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
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.
Lgtm, it's definitely easier to read! I had some suggestions you can take or leave
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.
Wow, big revamp :) The thing i actually had in mind for this Jira task was actually much smaller task--now that it's pure Ruby, was thinking the CONTRIBUTING doc could be revamped so that dev workflow does not need to be in a container. Yes, to run the regression tests it would still spin up containers, but simply to make a change, run a specific test case, and do a local build should be achievable directly on MacOS now, correct?
Oh right, yes, everything can run without container (although I still prefer container so it can enforce env isolation). I can add the additional context about running in host machine. |
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.
Thanks @xuan-cao-swi . i left a bunch of suggestions for the CONFIGURATION page, it's a nice revamp but i do feel AI assisted content gets kind of verbose and we still need to check for correctness.
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.
LGTM, thanks for the revisit!
Description
Test (if applicable)