-
Notifications
You must be signed in to change notification settings - Fork 32
Conversation
Not sure which emails we should list for conduct. Went with mine and @julik's for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primarily JS-isms in the CONTRIBUTING doc. Maybe we should have a look if some small nice Ruby projects have docs we could copy?
CONTRIBUTING.md
Outdated
[features requests](#feature-requests) and [submitting pull | ||
requests](#pull-requests), but please respect the following restrictions: | ||
|
||
* Please **do not** use the issue tracker for personal support requests (use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH we don't have enough exposure to divert to SO yet, so I would drop this clause until this becomes a necessity. If #21 ended up on SO I probably wouldn't even have found it, let alone used it as something actionable.
CONTRIBUTING.md
Outdated
project's developers might not want to merge into the project. | ||
|
||
Please adhere to the coding conventions used throughout a project (indentation, | ||
accurate comments, etc.) and any other requirements (such as test coverage). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's prudent to add that the project used Rubocop, which can be run with bundle exec rubocop -c .rubocop.yml
CONTRIBUTING.md
Outdated
# Clone your fork of the repo into the current directory | ||
git clone git@github.com:WeTransfer/eslint-config-wetransfer.git | ||
# Navigate to the newly cloned directory | ||
cd eslint-config-wetransfer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paths and stuff
CONTRIBUTING.md
Outdated
git checkout -b <topic-branch-name> | ||
``` | ||
|
||
4. Commit your changes in logical chunks. Please adhere to these [AngularJS Git Commit Message Conventions](https://docs.google.com/document/d/1QrDFcIiPjSLDn3EL15IJygNPiHORgU1_OOAqWjiDU5Y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would limit to "commit your changes in logical chunks or squash them for readability and conciseness" or some such. How Angular is relevant here I don't even know (as in: I do get it that we ported these conventions from the eslit-config repo but still). A link to a general tips page re. writing good commit messages could suffice IMO.
CONTRIBUTING.md
Outdated
Removing features on repo | ||
|
||
```bash | ||
git commit -m "refactor: message about this" -m "BREAKING CHANGE: message about the breaking change" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was never used on this repo, semver that we use implies that breaking changes are permitted as long as the major version is increased on released. I don't like this convention and I don't think we need it.
CONTRIBUTING.md
Outdated
Adding features on repo | ||
|
||
```bash | ||
git commit -m "feat: message about this feature" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See below (no "feat:", "churn:", "BREAKING CHANGE:" etc. commit message prefixes). There is a CHANGELOG for this.
|
||
## Enforcement | ||
|
||
Instances of abusive, harassing, or otherwise unacceptable behavior may be reported by contacting the project team at julik@wetransfer.com and/or noah@wetransfer.com. The project team will review and investigate all complaints, and will respond in a way that it deems appropriate to the circumstances. The project team is obligated to maintain confidentiality with regard to the reporter of an incident. Further details of specific enforcement policies may be posted separately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it that this gets handled via email first, instead of GH 👍
@julik Pushed some changes per your comments. :) |
@WJWH @felixbuenemann could you guys have a look? I would like to merge this soon. |
|
||
## Scope | ||
|
||
This Code of Conduct applies both within project spaces and in public spaces when an individual is representing the project or its community. Examples of representing a project or community include using an official project e-mail address, posting via an official social media account, or acting as an appointed representative at an online or offline event. Representation of a project may be further defined and clarified by project maintainers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that it is unlikely that there is a Website, Social Media Account or Official Email for a library, does it make sense to include the stuff about "public spaces" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The github repo is the de-facto website I'd think. Also online or offline event
could still apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's usefully harmless, in that if we want to give a talk about the library or reference it somewhere we might as well have the catch-all, even if it's not something we will likely encounter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I have no objections to keeping it as is.
CONTRIBUTING.md
Outdated
- [zip files](https://en.wikipedia.org/wiki/Zip_(file_format)) | ||
|
||
|
||
# How do I made a contribution? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: s/made/make/
CONTRIBUTING.md
Outdated
## Using the issue tracker | ||
|
||
The issue tracker is the preferred channel for [bug reports](#bug-reports), | ||
[features requests](#feature-requests) and [submitting pull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/features requests/feature requests/
CONTRIBUTING.md
Outdated
requests](#pull-requests), but please respect the following restrictions: | ||
|
||
* Please **do not** derail or troll issues. Keep the discussion on topic and | ||
respect the opinions of others. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add link to CoC here?
CONTRIBUTING.md
Outdated
|
||
## Bug reports | ||
|
||
A bug is a _demonstrable problem_ that is caused by the code in the repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd drop "the" at the beginning of "the code in the repository" for better readability.
CONTRIBUTING.md
Outdated
> 2. This is the second step | ||
> 3. Further steps, etc. | ||
> | ||
> `<url>` - a link to the reduced test case, if possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be useful to add a linked reference to [Gist](https://gist.github.com)
as a way to share test cases.
CONTRIBUTING.md
Outdated
|
||
## Pull requests | ||
|
||
Good pull requests - patches, improvements, new features - are a fantastic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace n-dashes with m-dashes. You don't need to use —
on macOS they can be entered using Cmd+-
.
CONTRIBUTING.md
Outdated
otherwise you risk spending a lot of time working on something that the | ||
project's developers might not want to merge into the project. | ||
|
||
Please adhere to the coding conventions used throughout a project (indentation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/a project/the project/
CONTRIBUTING.md
Outdated
Please adhere to the coding conventions used throughout a project (indentation, | ||
accurate comments, etc.) and any other requirements (such as test coverage). | ||
|
||
The project uses Rubocop which can be run using `bundle exec rubocop -c .rubocop.yml`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
The project uses Rubocop to enforce a consistent code style.
You can runbundle exec rubocop
from the root of the repository to see what needs to be fixed.
I think specifying -c .rubocop.yml
is redundant or does it avoid using a user specific config?
Also if we mention how to run rubocop, should we mention how to run rspec here as well since the previous paragraph mentions test coverage?
CONTRIBUTING.md
Outdated
with a clear title and description. | ||
|
||
**IMPORTANT**: By submitting a patch, you agree to allow the project owner to | ||
license your work under the same license as that used by the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a reference to the license, like:
See LICENSE.txt at the top of the repository for details.
I have submitted a review with a bunch of suggested changes, none of these are blockers for merging. |
|
||
Instances of abusive, harassing, or otherwise unacceptable behavior may be reported by contacting the project team at julik@wetransfer.com and/or noah@wetransfer.com. The project team will review and investigate all complaints, and will respond in a way that it deems appropriate to the circumstances. The project team is obligated to maintain confidentiality with regard to the reporter of an incident. Further details of specific enforcement policies may be posted separately. | ||
|
||
Project maintainers who do not follow or enforce the Code of Conduct in good faith may face temporary or permanent repercussions as determined by other members of the project's leadership. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice this would involve removing commit rights, right? Why not just say that instead of leaving it at some nebulous "consequences"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Planning for a worst-case scenario - we want to keep it vague so people don't think "Oh, I can be terrible to other users and all they'll do is remove my commits."
CONTRIBUTING.md
Outdated
|
||
A good bug report shouldn't leave others needing to chase you up for more | ||
information. Please try to be as detailed as possible in your report. What is | ||
your environment? What steps will reproduce the issue? What browser(s) and OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Have pushed some updates. Thanks much everyone. I'll take a look at creating a sample spec script, but considering that we refer to the contents of the |
I think we can do that later. The |
Sounds good to me. Are we happy with what we have now? |
Not sure which emails we should list for contact purposes. Went with mine and @julik's for now. Also added a rudimentary set of contributor guidelines based on our eslint config repo.
Closes #29.