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

Add CONTRIBUTING.md to root folder #6070

Merged
merged 2 commits into from
May 30, 2017

Conversation

LourensPool
Copy link
Contributor

@LourensPool LourensPool commented Mar 13, 2017

In a response to #3368 I created the file CONTRIBUTING.md and placed it in the root folder.

I think adding the file helps in realizing smooth pull requests.

Any suggestions/opinions to CONTRIBUTING.md are welcome :)

@mastrolinux mastrolinux added the in progress Work on this item is in progress label Mar 13, 2017
@per1234
Copy link
Collaborator

per1234 commented Mar 13, 2017

I think this is extremely important but I think it's essential to provide more rules for issues. There are so many low quality issues created in this repository that I think some clearly stated rules could save the developers a lot of time that could be spent more productively. Here are my suggestions for additions:

Years of working with Arduino users tells me that most people submitting issue reports will ignore the "Please review the guidelines for contributing to this repository." notice. To get better compliance I recommend also adding an ISSUE_TEMPLATE.md that says something like:

Read the rules at the "guidelines for contributing" link above before submitting an issue report.

I think it would actually be better to move the specific rules from the Development Policy wiki page to CONTRIBUTING.md, while leaving the general development philosophy on the wiki page.

There is also the option of putting CONTRIBUTING.md and ISSUE_TEMPLATE.md in a .github folder in the repository root but I think it's fine in the root.

CONTRIBUTING.md Outdated
@@ -0,0 +1,65 @@
## Contributing guide
A simple four step guide to consider when you want your pull request to be merged without a hassle. This guide mainly focusses on the proper use of Git. It has some overlap with the more general information found in the [Development Policy File](https://github.com/arduino/Arduino/wiki/Development-Policy).
Copy link
Collaborator

Choose a reason for hiding this comment

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

"focusses" should be spelled "focuses"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I fixed it.

CONTRIBUTING.md Outdated
3. Capitalize the subject line
4. Do not end the subject line with a period `(.)`
5. Use the imperative mood in the subject line.
This should be in the written as giving an instruction for example "update getting started documentation" (it shows what the PR achieves when merging it)
Copy link
Collaborator

Choose a reason for hiding this comment

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

"update" should be capitalized (see point 3). I actually don't think this is the ideal example of a subject because the term "update" is much too general.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I changed it to "Fixed save-as bug"

@matthijskooijman
Copy link
Collaborator

We should really have such a file, it's been on my personal wishlist for quite a while. @per1234 agreed with your suggestions, those are really important things to mention, even more so than how to create a PR. I do think your suggestions need a bit more structure to become more clear (e.g. "what this tracker is for", "what this tracker is not for", "Before submitting an issue", "Before submitting a PR", etc.

@per1234
Copy link
Collaborator

per1234 commented Mar 14, 2017

I agree about the structure/organization. That list I made was just some brainstorming of things that should be included rather than a suggestion of the actual format. I think it would be good to have a section of the document for issue rules and one for PR rules so the person reading them can quickly go to the ones that are relevant to what they want to do.

@LourensPool
Copy link
Contributor Author

Thanks for the feedback and information.

I updated the document. I used a checkbox instead of a bullet list to allow the reader to check off all steps he took.

The document can still be more structured. But I also like the fast readability of this approach.

An ISSUE_TEMPLATE.md like @per1234 proposed is a good idea as well.

@facchinm facchinm added this to the Release 1.8.3 milestone Mar 20, 2017
@LourensPool
Copy link
Contributor Author

@matthijskooijman @per1234 @facchinm

Any ideas on how to move forward with these documents ?

@per1234
Copy link
Collaborator

per1234 commented Mar 23, 2017

I still think it would be better to organize the information in two separate parts: issues and pull requests. This will make it very easy for someone to see which parts they need to read and which they can skip over. As it is now it looks at a first glance that you should start at point 1 and then proceed through point 6 but actually someone wanting to submit an issue would just need to read 1 and 2 and someone wanting to submit a pull request would only need to read 3-6. It would only take a few seconds of for this to become apparent to the reader but at the first glance it seems more formidable than it actually is.

I don't think there's any way for the user to actually check the checkboxes unless they have printed the document or are editing the markdown. So maybe bullet points would be better. On the other hand, maybe the checkboxes emphasize that each point should be focused on and mentally checked off rather than just skimmed over. I don't have strong feelings about that topic either way.

I had mentioned in my first comment that it's worth considering moving some of the pull request rules from the Development Policy page. These are the items that stand out to me:

  • Match the indentation and style of the rest of the file.
  • Don't include a commented-out version of the previous version of the code.
  • Don't add a comment with your name and the date of changes.
  • Please note that the Arduino core libraries support many boards and processors. When fixing or adding functionality for one of them, it's easy to break something on the others. Please test your changes on as many processors as possible. Even if you don't have a particular board, try compiling your patch for it to make sure that you haven't introduced any errors.

The concern with adding even more information is that it can accumulate to an overwhelming amount of text, rather than an easy to read list of a few of the most important concepts. I do think someone submitting a pull request will be more motivated to spend a little extra time making sure it's done right since they have already demonstrated a level of commitment by writing the patch.

You did a great job of writing this document. I appreciate you incorporating my suggestions. I think this information is very valuable, not only for contributing to this project but for generally understanding how to effectively work with code repositories.

@cmaglie
Copy link
Member

cmaglie commented May 29, 2017

I like the document, but also I'd like to add the changes suggested by @per1234. Maybe a way to move forward is to merge as it is now and edit later (since probably the current version of the contributing guide is better than nothing)?

@per1234
Copy link
Collaborator

per1234 commented May 29, 2017

I agree that it's worth merging this as-is to get something in place immediately. Once that happens I'm happy to submit a PR for my suggested changes.

@cmaglie cmaglie merged commit 666ac02 into arduino:master May 30, 2017
@cmaglie cmaglie removed the in progress Work on this item is in progress label May 30, 2017
@cmaglie
Copy link
Member

cmaglie commented Jun 6, 2017

I'm really happy with the text, but after reading some of the recent posts on I have to say that is not so effective as I hoped... maybe the small link on the top of the "new issue" form is not enough to catch the eyes...

@per1234
Copy link
Collaborator

per1234 commented Jun 6, 2017

@cmaglie I agree, the message that GitHub displays is too easy for the typical Arduino issue reporter to ignore. What do you think about my proposal to add an ISSUE_TEMPLATE.md file? Here's an example of how it looks in one of my repositories:
https://github.com/per1234/WatchdogLog/issues/new
It's a little more in their face because the text is right there in the input field they need to use to write the issue. I'd be happy to submit a pull request for that.

Sorry I haven't gotten around to submitting a PR for my CONTRIBUTING.md improvements proposal yet. I'll try to get to that today.

@carlosperate
Copy link
Contributor

If adding a template I'd be tempted to suggest to add a quick line saying something along the lines "please read the contributing file and ensure you meet all the checklist criteria" and right below it add the checklist from CONTRIBUTING.md, "forcing" the users to check all items as an agreement that they have read the documentation and followed those steps.

@per1234 per1234 mentioned this pull request Jun 7, 2017
@matthijskooijman
Copy link
Collaborator

About adding an ISSUE_TEMPLATE.md, I just noticed github also supports multiple templates, and then offers a choice of them, which is probably useful for Arduino to. For an example, see https://github.com/pypa/pipenv/issues/new/choose

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.

None yet

7 participants