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

Contributing guidelines #514

Open
gynter opened this issue Aug 4, 2017 · 2 comments
Open

Contributing guidelines #514

gynter opened this issue Aug 4, 2017 · 2 comments

Comments

@gynter
Copy link
Collaborator

gynter commented Aug 4, 2017

Hello!

Since we now have more collaborators I suggest us to settle on somekind contributing guidelines.

My suggestions:

  • Use English in commit messages, branch, and tag names;
  • Branch and tag names must be separated by dash -;
  • Follow the seven rules of a great git commit message:
    • Separate subject from body with a blank line;
    • Limit the subject line to 50 characters;
    • Capitalize the subject line;
    • Do not end the subject line with a period;
    • Use the imperative mood in the subject line;
    • Wrap the body at 72 characters;
    • Use the body to explain what and why vs. how;
  • Pull requests must have it's own feature branch;
  • All commits in pull request must be rebased and squashed, except
    when the commits are required to mark specific advancement (i.e
    collection of serveral major features) in the development history;
  • Merge conflicts must be resolved in feature branch during the rebase;
  • Issues may be closed via commit messages using fix keyword i.e fix
    Added Hikari #24 closes issue Added Hikari #24. Multiple issues must be separated by comma like
    this: fix Added Hikari #24, fix Added theme: Architect #32, fix Adding Jekyll Metro theme #51.
  • Exceptions can be made for theme pull requests only. All changes by collaborators must follow the rules above;
  • When merging pull request, merger must squash the commits and follow the rules when commiting;
  • No direct commits to master branch, everything goes via pull requests;
  • Any major changes to site layout and functionality must be firstly discussed in issue/pull request.

The point of the last one is, that this site has always been built on keep it simple, stupid concept, therefore adding awesome bells and whistles isn't always a good idea.

Squashing can be both done via CLI or Github web interface.

All suggestions and counter arguments are welcome!

@gynter gynter changed the title Contribution guidelines Contributing guidelines Aug 4, 2017
@insipx
Copy link
Collaborator

insipx commented Aug 4, 2017

Good idea IMO. Would solidify what we the collaborators need to do, as well as make it clearer how users are to make their listing/pull request.

Also, not cluttering up the master branch with dozens of random commits and instead rebasing+squashing would make the repository easier to manage in the future.

In addition ( I don't know if this is the case currently) I believe we should require a 'review' from another collaborator in order for one collaborator to merge a PR they made. This would prevent collaborators arbitrarily submitting and merging their own themes. In my short stay as a collaborator, I haven't made any PR's, so I don't know if this is the case as of now. But if it is not, I think it would be beneficial to make it so, as well as remove direct-commit permission from the collaborators to the master branch.

@mattvh
Copy link
Owner

mattvh commented Aug 10, 2017

I think there are some good points in there. I'm generally in favour of having some contribution guidelines. Not necessarily too strict, but strongly suggested anyway. Just some initial thoughts:

Use English in commit messages, branch, and tag names;

Definitely.

Branch and tag names must be separated by dash -;

I'm okay with that, though I personally have a habit of naming branches like "somenewbranch," which I'd see as acceptable. I'm okay with standardising on dashes for when separators are used, though.

Follow the seven rules of a great git commit message

This is one I'd see as a suggestion and not a firm requirement, so long as commit messages aren't absolutely terrible. I generally settle for "descriptive and informative with acceptable spelling/grammar." e.g. I prefer past tense subject lines to imperative myself, but find them both equally acceptable. A policy requiring messages to be reasonably well formatted and useful definitely has a plus-one from me though.

All commits in pull request must be rebased and squashed, except when the commits are required to mark specific advancement (i.e collection of serveral major features) in the development history;

That sounds acceptable. I'd also clarify that additional commits for updates to the PR (e.g. requested changes) should be fine.

When merging pull request, merger must squash the commits and follow the rules when commiting;

This could be enforced through repository settings, I believe, if it's desirable: https://github.com/blog/2141-squash-your-commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants