-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Add contributing dir with Config file checklist #7017
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.
Overall, I like this idea a lot. I think it will help distribute knowledge and enable better contributions.
I especially thought the explanations of why/when you would need to go through each of the four cases presented was useful to know whether I needed to do it or not.
I had to do the first 2 steps recently with hclog, so I mimicked running through this checklist with that change. I found that using a gist was not the greatest UX, since it only showed me the raw markdown. I ended up copying it again in a comment below (https://gist.github.com/crhino/6cefaa4a1f06aebbcc3ef274f3b06b58). Not sure whether to keep that suggestion in or not.
I also liked having the exact file locations pointed out, and I added a few comments updating some of the bullet points with full paths.
Co-Authored-By: Chris Piraino <[email protected]>
Great feedback @crhino thanks!
If you name the "file" something that ends in Do you think it would help to be more explicit about that in the README? |
Ah, good to know that gists can do that! I don't think it is necessary to go into that much detail in our README, the essential point is to include the checklist with the PR in my opinion, the workflow used to get there doesn't have to be too detailed. |
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
This checklist was reviewed internally in a document form a few months ago but I wanted to make it public.
I've followed Nomad's conventions of adding a
contributing
dir. The only quirk here is we already have a.github/CONTRIBUTING.md
which has some overlap. GitHub links to that in new PRs so it's useful to surface there and will only pick it up from.github
in the root or in/docs/
dir. I don't want to add/docs
and choose not to move it for now as we don't want other checklists in the root so it still doesn't end up in the same place but open to other options here.I tried to make this a relatively minimal change and we can certainly iterate on the README and other things.
The
contributing/README.md
is based heavily on the Nomad one: https://github.com/hashicorp/nomad/tree/master/contributing for now. I don't know that they need to be in perfect harmony so we can iterate but it seemed like a decent starting point.Finally I moved
INTERNALS.md
to avoid having a sprawl of things in different places I left a stub behind so links don't break in case anyone links to it in mailing list issues etc.