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.md #6357

Merged
merged 11 commits into from
May 7, 2019
Merged

Improve CONTRIBUTING.md #6357

merged 11 commits into from
May 7, 2019

Conversation

per1234
Copy link
Collaborator

@per1234 per1234 commented Jun 7, 2017

These are the changes I proposed in #6070

Since I made some significant changes, I broke it into multiple commits to make it easy to modify the PR if some of my edits are rejected. I am happy to squash this to a single commit before the merge.

I welcome any feedback on my edits and am happy to make any changes requested.

CONTRIBUTING.md Outdated
Arduino Forum issues | [arduino/forum-issues](https://github.com/arduino/forum-issues)
Arduino libraries | [arduino-libraries](https://github.com/arduino-libraries)
arduino-builder | [arduino/arduino-builder](https://github.com/arduino/arduino-builder)
[Arduino Web Editor](https://create.arduino.cc/editor) | [arduino/arduino-create-agent](https://github.com/arduino/arduino-create-agent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cmaglie, @mastrolinux, is this indeed the right place to report Create issues?

Copy link
Collaborator Author

@per1234 per1234 Jun 15, 2017

Choose a reason for hiding this comment

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

I see there is an empty repository: https://github.com/arduino/arduino-create but there is no explanation of the purpose. I was told there is a private issue tracker and some plans to create a public one but I haven't heard anything about it since. I realize that the arduino-create-agent repository is not the ideal place for general Arduino Web Editor issue reports unrelated to the agent.

EDIT: The question has now been asked here: https://github.com/arduino/arduino-create/issues/1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have now updated this to a link to the Create > Editor forum section according to https://github.com/arduino/arduino-create/issues/1#issuecomment-351344954

Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

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

The new version looks ok to me, but some parts of the old version might also still be useful. In particular, the old version looks a bit more structured to me, the new version is a bit more a long list of bullet points.

Also, I'm not sure if a Issues/PR division is the best overall structure. That kind of assumes someone already knows that they want to submit either one, whereas the people that need these guidelines most probably know they have a "bug", "problem", "feature request", "need help", etc. So perhaps a bit of an introduction that identifies a few cases, explains when to use an issue, PR or go elsewhere, before going into detail about issues might help?

CONTRIBUTING.md Outdated
[Arduino Web Editor](https://create.arduino.cc/editor) | [arduino/arduino-create-agent](https://github.com/arduino/arduino-create-agent)
Arduino SAMD Boards (Zero, MKR1000, MKRZero, etc.) | [arduino/ArduinoCore-samd](https://github.com/arduino/ArduinoCore-samd)
Arduino SAM Boards (Due) | [arduino/ArduinoCore-sam](https://github.com/arduino/ArduinoCore-sam)
Arduino's build of AVRDUDE | [arduino/avrdude-build-script](https://github.com/arduino/avrdude-build-script)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably also include gcc and avr-libc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I see now, that repository was in the original version of CONTRIBUTING.md and I accidentally omitted it from my table. I've added it back in now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I somehow thought that the toolchain repo did both gcc and avrdude, but this is indeed about avrdude. Adding both separately seems the good approach indeed.

Arduino SAM Boards (Due) | [arduino/ArduinoCore-sam](https://github.com/arduino/ArduinoCore-sam)
Arduino's build of AVRDUDE | [arduino/avrdude-build-script](https://github.com/arduino/avrdude-build-script)
3rd party libraries, hardware, or sketches | Report issues to the author of the software, *not* Arduino.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps a remark like below should be good here?

When you're not sure where your issue belongs, report it at arduino/Arduino and we'll move it to where it belongs" (but remember: Only bug reports and feature requests, do not ask for help with your own code there).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

CONTRIBUTING.md Outdated
- Code formatting should be consistent with the style used in the existing code.
- Don't leave commented out code. A record of this code is already preserved in the commit history.
- 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.
- All commits must be atomic. This means that the commit completely accomplishes a single task. Each commit should result in fully functional code. Multiple tasks should not be combined in a single commit. For more information see http://www.freshconsulting.com/atomic-commits.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps say: Multiple tasks should not be combined in a single commit, but a single task should not be split over multiple commits (e.g. one commit per file modified is not good practice).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you requesting that your suggested text replace the text of that bullet point completely, removing any mention of the term "atomic"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, just extend/replace the "Multiple tasks should not be combined in a single commit" sentence.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the clarification. Done!

CONTRIBUTING.md Outdated
- Completely explain the purpose of the commit. Include rationale for the change, any caveats, side-effects, etc.
- If your pull request fixes an issue in the issue tracker, use the [closes/fixes/resolves syntax](https://help.github.com/articles/closing-issues-via-commit-messages) in the body to indicate this.
- See http://chris.beams.io/posts/git-commit for more tips on writing good commit messages.
- Rebasing pull requests is OK and encouraged. After submitting your pull request some changes may be requested. Rather than adding unnecessary extra commits to the pull request you can squash these changes into the existing commit and then do a force push to your fork. Leave a comment on the pull request thread to explain that the history has been changed. This will help to keep the commit history of the repository clean.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps add: When you do a force push to your fork, the PR will be updated with your new changes, so there is no need to open a new PR to make changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

CONTRIBUTING.md Outdated
- Check if your issue has already been resolved in the [hourly build](http://www.arduino.cc/en/Main/Software#hourly).
- Submit issue reports to the correct repository:

Issue subject | Report at
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm missing translations from this list. IIRC there is a separate system for that (transifex?) somewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added a translations row to the table with a link to the appropriate transifex link.

@per1234
Copy link
Collaborator Author

per1234 commented Jun 15, 2017

In particular, the old version looks a bit more structured to me, the new version is a bit more a long list of bullet points.

I have to disagree there. I found the other version to contain the necessary information but very lacking in organization. That was essentially the whole point of my rewrite. This is very much a subjective thing and my point of view on may be uncommon. I realize this PR represents a major structural reorganization but I just wasn't happy with the previous structure. If the structural changes I've made are not considered desirable then I'm happy to modify this PR to use the original structure with the addition of the minor content additions I've made.

Also, I'm not sure if a Issues/PR division is the best overall structure. That kind of assumes someone already knows that they want to submit either one, whereas the people that need these guidelines most probably know they have a "bug", "problem", "feature request", "need help", etc. So perhaps a bit of an introduction that identifies a few cases, explains when to use an issue, PR or go elsewhere, before going into detail about issues might help?

I was thinking of this file being viewed by people who clicked the link in the "Before you submit an issue, please review the guidelines for this repository." or "Before you create a pull request, please review the guidelines for contributing to this repository." notices that GitHub displays when you click the "New Issue" button or start a pull request. In that case the user already knows which one they're doing so it makes perfect sense to organize the document in this way. However, what I think you're bringing to my attention is that this document has value beyond that usage as a general guide to contributing. I think that's an excellent point. I'll see what I can do to incorporate this suggestion.

@matthijskooijman
Copy link
Collaborator

I was thinking of this file being viewed by people who clicked the link in the "Before you submit an issue, please review the guidelines for this repository." (...) In that case the user already knows which one they're doing (...)

That is a good point I didn't realize yet, but experience shows that users that report issues do not always actually know what they're doing :-p And indeed, this document could/should have a more generic value and can be linked from other places as well.

The previous numbered format of the file made it difficult to determine which sections needed to be read depending on whether an issue report or a pull request was being submitted. This could make the task of reading the file look more daunting than it actually was.

Closes #3368
@per1234
Copy link
Collaborator Author

per1234 commented Jun 16, 2017

So perhaps a bit of an introduction that identifies a few cases, explains when to use an issue, PR or go elsewhere, before going into detail about issues might help?

I've made an attempt at this in 7f718da. After some unsuccessful attempts at writing the information in sentence form I resorted to a table. A benefit of this is I was able to move the non-issue rows of the table in the Issues section of the document to the contribution options table. However, the table coming so soon in the document might make it less approachable so maybe it needs another sentence or two before the table to flesh out the introduction. I'm currently having a bit of a writers block there.

I also added an introduction to the Pull Requests section in pursuit of matthijskooijman's suggestion that I quoted above.

@per1234
Copy link
Collaborator Author

per1234 commented Jun 17, 2017

I added "Monetary" to the list of contribution options with links to the donation page and the Arduino store.

@per1234
Copy link
Collaborator Author

per1234 commented Jun 17, 2017

I added "Arduino AVR Boards" to the list of valid issue topics for the arduino/Arduino repository.

@per1234
Copy link
Collaborator Author

per1234 commented Jun 20, 2017

I have a question. Supposedly the two Arduino companies are unified now and I had expected that would mean the separate arduino.org website would be taken down but this hasn't happened.

  • Should arduino.org problems be added to the list of valid topics for the arduino/Arduino issue tracker?
  • Should the arduino.org forum be added to the list of valid topics for the arduino/forum-issues issue tracker?

EDIT: Now Solved!

@facchinm facchinm added this to the Release 1.8.4 milestone Jul 25, 2017
@per1234
Copy link
Collaborator Author

per1234 commented Aug 18, 2017

Updates:

  • Request descriptive issue/PR titles
  • Exclude forum from list of valid arduino.cc issues for the arduino/Arduino repository.

@per1234
Copy link
Collaborator Author

per1234 commented Aug 18, 2017

Question:
Should issues about library documentation on the arduino.cc reference pages be submitted to the relevant library repository or the arduino/Arduino repository?

The former seems more sensible to me, more so since the roadmap is to eventually store the documentation in the library repositories. However, it does seem more common to report this sort of issue to the arduino/Arduino repository and issues submitted to this repository seem to get more official attention.

The current wording of CONTRIBUTING.md (the official version or my modified version both) would probably be interpreted as saying that arduino/Arduino should be used but it's not entirely clear.

The checkboxes format previously used is confusing because the checkboxes can't actually be checked.
- Attempt to incorporate more of the content from:
  - https://github.com/arduino/Arduino/wiki/Development-Policy
  - #3368 (comment)
  - #3337 (comment)
- Fix typos
- Change example commit title to actually use imperative mode
- I interpreted @matthijskooijman's comment about rebasing as referring to squashing fixup commits rather than resolving merge conflicts and edited accordingly.
This serves as an introduction to the document.
The reference-en repository is now being used to generate the Language Reference pages. The Language Reference translation projects also seem to be progressing.
@per1234
Copy link
Collaborator Author

per1234 commented Mar 1, 2018

I force pushed some minor improvements:

  • Wording changes
  • Issues repo table links directly to issue issue tracker pages rather than repo home page.
  • Language reference link to list of all reference repos instead of arduino/reference-en
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -8,7 +8,7 @@ Thanks for your interest in contributing to this free open source project! Ardui
 | - Bug fix<br/>- Enhancement | Pull Request (read the [pull request guidelines](#pull-requests)) |
 | [Arduino Playground](http://playground.arduino.cc/) | This is a publicly editable wiki. Feel free to make edits or create a post on the [forum](http://forum.arduino.cc/index.php?board=24.0) to discuss changes. |
 | Translations for the Arduino IDE | [transifex](https://www.transifex.com/mbanzi/arduino-ide-15/) |
-| Translations for the [Language Reference](https://www.arduino.cc/reference) | [arduino/reference-{language code}](https://github.com/arduino?q=reference-) |
+| Translations for the [Language Reference](https://www.arduino.cc/reference) | [Reference repositories](https://github.com/arduino?q=reference-) |
 | Monetary | - [Donate](https://www.arduino.cc/en/Main/Contribute)<br/>- [Buy official products](https://store.arduino.cc) |
 
 
@@ -19,17 +19,17 @@ Thanks for your interest in contributing to this free open source project! Ardui
 
 | Issue topic | Report at |
 |-|-|
-| Arduino IDE, arduino.cc (but not the Language Reference, Arduino Playground, or forum), Library Manager additions | [arduino/Arduino](https://github.com/arduino/Arduino) |
-| [Language Reference](https://www.arduino.cc/reference) | [arduino/reference-en](https://github.com/arduino/reference-en) |
-| Arduino Forum issues | [arduino/forum-issues](https://github.com/arduino/forum-issues) |
+| Arduino IDE, Arduino AVR Boards, arduino.cc (but not the Arduino Playground or forum), Library Manager additions | [arduino/Arduino](https://github.com/arduino/Arduino/issues) |
+| [Language Reference](https://www.arduino.cc/reference) | [Reference repositories](https://github.com/arduino?q=reference-) |
+| Arduino Forum | [arduino/forum-issues](https://github.com/arduino/forum-issues/issues) |
 | Arduino libraries | [arduino-libraries](https://github.com/arduino-libraries) |
-| arduino-builder | [arduino/arduino-builder](https://github.com/arduino/arduino-builder) |
-| [Arduino Web Editor](https://create.arduino.cc/editor) | [Create > Editor section of the Arduino Forum](http://forum.arduino.cc/index.php?board=101.0) |
-| Arduino AVR Boards (Uno, Mega, Leonardo, etc.) | [arduino/ArduinoCore-avr](https://github.com/arduino/ArduinoCore-avr) |
-| Arduino SAMD Boards (Zero, MKR1000, MKRZero, etc.) | [arduino/ArduinoCore-samd](https://github.com/arduino/ArduinoCore-samd) |
-| Arduino SAM Boards (Due) | [arduino/ArduinoCore-sam](https://github.com/arduino/ArduinoCore-sam) |
-| AVR Toolchain for Arduino | [arduino/toolchain-avr](https://github.com/arduino/toolchain-avr) |
-| Arduino's build of AVRDUDE | [arduino/avrdude-build-script](https://github.com/arduino/avrdude-build-script) |
+| arduino-builder | [arduino/arduino-builder](https://github.com/arduino/arduino-builder/issues) |
+| [Arduino Web Editor](https://create.arduino.cc/editor) | [**Create > Editor** section of the Arduino Forum](http://forum.arduino.cc/index.php?board=101.0) |
+| Arduino AVR Boards (Uno, Mega, Leonardo, etc.) | [arduino/ArduinoCore-avr](https://github.com/arduino/ArduinoCore-avr/issues) |
+| Arduino SAMD Boards (Zero, MKR1000, MKRZero, etc.) | [arduino/ArduinoCore-samd](https://github.com/arduino/ArduinoCore-samd/issues) |
+| Arduino SAM Boards (Due) | [arduino/ArduinoCore-sam](https://github.com/arduino/ArduinoCore-sam/issues) |
+| AVR Toolchain for Arduino | [arduino/toolchain-avr](https://github.com/arduino/toolchain-avr/issues) |
+| Arduino's build of AVRDUDE | [arduino/avrdude-build-script](https://github.com/arduino/avrdude-build-script/issues) |
 | 3rd party libraries, hardware, or sketches | Report issues to the author of the software, *not* Arduino. |
 
 When you're not sure where your issue belongs, report it at [arduino/Arduino](https://github.com/arduino/Arduino) and we'll move it to where it belongs (but remember: Only bug reports and feature requests, do not ask for help with your own code there).
@@ -37,8 +37,8 @@ When you're not sure where your issue belongs, report it at [arduino/Arduino](ht
 - Search [existing pull requests and issues](https://github.com/arduino/Arduino/issues?q=) to be sure it hasn't already been reported. If you have additional information to provide about an existing issue then please comment on that issue. If you simply want to express your support then use the [Reactions feature](https://github.com/blog/2119-add-reactions-to-pull-requests-issues-and-comments).
 - State the newest version of the Arduino IDE you have verified the issue with and which operating system you are using.
 - The issue title should be concise yet descriptive. Vague titles make it difficult to know the purpose of the issue when looking through the list of reports and may cause your issue to not be given proper attention.
-- Describe the issue and what behavior you were expecting. Post complete error messages using [markdown code fencing](https://guides.github.com/features/mastering-markdown/#examples).
-- Provide a full set of steps necessary to reproduce the issue. Demonstration code should be complete, correct, and simplified to the minimum amount of code necessary to reproduce the issue. Please use [markdown code fencing](https://guides.github.com/features/mastering-markdown/#examples) when posting code.
+- Describe the issue and what behavior you were expecting. Post complete error messages using [Markdown code fencing](https://guides.github.com/features/mastering-markdown/#examples).
+- Provide a full set of steps necessary to reproduce the issue. Demonstration code should be complete, correct, and simplified to the minimum amount of code necessary to reproduce the issue. Please use [Markdown code fencing](https://guides.github.com/features/mastering-markdown/#examples) when posting code.
 - Be responsive. We may need you to provide more information, please respond as soon as possible.
 - If you find a solution to your problem update your issue report with an explanation of how you were able to fix it and close the issue.
 - Library Manager submissions: make sure your library meets all the requirements listed in the [Library Manager FAQ](https://github.com/arduino/Arduino/wiki/Library-Manager-FAQ).

…RIBUTING.md

Arduino decided to make the Arduino Playground read only, so it is no longer possible for people to edit it directly. They will now need to report issues or suggestions for improvement to the arduino/Arduino issue tracker so that someone with edit privileges can make the required changes.
@pnndra pnndra merged commit 0119fe3 into arduino:master May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Work on this item is in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants