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

Improve Contributing #3189

Merged
merged 1 commit into from
Sep 17, 2016
Merged

Conversation

scott-w
Copy link
Member

@scott-w scott-w commented Sep 17, 2016

Proposed changes

  • Change the branch choice docs
  • Added approval process
  • Put the PR process into a top-level header

Link to the issue: #3185

I've re-written some of the contributor file to try and cover some of the changes to our process.
@scott-w scott-w added the docs label Sep 17, 2016
@scott-w scott-w mentioned this pull request Sep 17, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b910a00 on scott-w:contributor-improvement-2 into 828a5cc on marionettejs:master.

Please remember that Marionette is a community-maintained project and, as such,
many of us are working on this in our spare time. If we haven't commented on
your pull request, please be patient. We may be available on our Gitter channel
to discuss further.
Copy link
Member

Choose a reason for hiding this comment

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

Good remark 👍

3. [Running Tests](#running-tests)
4. [Updating docs](#updating-docs)
3. [Submitting patches and fixes](#submitting-patches-and-fixes)
4. [Running Tests](#running-tests)

Copy link
Member

Choose a reason for hiding this comment

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

Some titles are missed:

  • When you don't have a bug fix
  • Determining your branch
  • Submitting a Great Patch
  • Solving Issues
  • Coding Guidelines
  • How we Approve Pull Requests

Maybe the were not added in purpose.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to stick to the top-level headers here so the reader could quickly find the thing they're currently working on e.g. they're submitting a pull request.

That said, I can definitely see the arguments for including them as someone might be thinking "what do I do for docs!"

Copy link
Member

Choose a reason for hiding this comment

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

Got your point.

@denar90 denar90 merged commit 4da2b32 into marionettejs:master Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants