Skip to content
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

Cleanup contributor and collaborator guides #657

Merged
merged 2 commits into from
Apr 18, 2016

Conversation

phillipj
Copy link
Member

Noticed we've got some issues with our current .md guides:

  • CONTRIBUTING.md has the contents of a collaborator guide, therefor it's renamed to COLLABORATOR_GUIDE.md
  • Copied the baseline CONTRIBUTING.md from TSC/BasePolicies, changing TC -> WG in the last section, which seems more inline with what we've got here

As the contents clearly states this is the collaborator guide.
@lpinca
Copy link
Member

lpinca commented Apr 18, 2016

Should we keep line length to 80 chars where possible? Apart from this LGTM.

@fhemberger
Copy link
Contributor

@lpinca Personally, I'm not entirely happy with this 80-chars-thingie in markdown files. It makes diffs harder to compare, when you have to hard-wrap a whole paragraph again, because you added/deleted only a couple of words. Editors can soft-wrap the content.

@lpinca
Copy link
Member

lpinca commented Apr 18, 2016

No problem, I added that comment because that's how most of the markdown files are currently formatted.

Edit: Actually there isn't a consistent style, so ignore everything I wrote.

@fhemberger
Copy link
Contributor

I'm also not sure what "best practice" is in this case, but as rendered Markdown output is soft-wrapped as well and GitHub does a pretty good job with wrapping the content of diffs, I don't think we really need this 80 chars limit.


* [Issues and Pull Requests](#issues-and-pull-requests)
* [Accepting Modifications](#accepting-modifications)
- [Involving the Website Admin Group](#involving-the-website-admin-group)
Copy link
Contributor

@fhemberger fhemberger Apr 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* for consistency, or it this a sub-item of "Accepting Modifications"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or it this a sub-item of "Accepting Modifications"?

yep

Copied from [TSC/BasePolicies](https://github.com/nodejs/TSC/tree/master/BasePolicies) with changes
related to the working group's TC, as we don't have a TC for the website.
@phillipj phillipj force-pushed the cleanup-collab-contrib-guide branch from 872be6f to 4c0e7ed Compare April 18, 2016 16:02
@phillipj
Copy link
Member Author

Thanks for reviewing! Replaced committer -> collaborator and some of TC -> WG.

PTAL

@fhemberger
Copy link
Contributor

LGTM.

@fhemberger fhemberger merged commit 05ef4f2 into nodejs:master Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants