Add to contributing guidelines, especially about git/github#215
Add to contributing guidelines, especially about git/github#215
Conversation
| **Write many small commits** each containing an atomic change instead of one | ||
| large mega-commit. This not only makes code review easier, but it also makes | ||
| commits that show up in ``git blame`` 10 years from now make more sense. | ||
| Strive to **write many small commits** each containing an atomic change instead |
There was a problem hiding this comment.
However, small commits makes not clear what is a complete feature.
In other projects, any commit should pass all tests, and if adding a new feature, it should contain also the test. Following that, commits can be actually quite big.
So far i never heard of a workflow that would prioritize git blame. If a project is mature/in production, developers try to modify code making as less changes as possible to other parts of the code (probably made by other developers).
| of one large mega-commit. This not only makes code review easier, but it also | ||
| makes commits that show up in ``git blame`` 10 years from now make more sense. | ||
|
|
||
| **Prefer a rebase workflow instead of merge**. Incorporating PRs should be done |
There was a problem hiding this comment.
Rebase workflow makes commits cleaner, however, unless done in different branches as you commente below, it would imply "lying" for remote branches.
| not merge master into your topic branch**; instead, rebase your topic branch on | ||
| top of master or cherry-pick the changes. | ||
|
|
||
| **Do not force push lightly** unless branches are clearly labeled as ones that |
There was a problem hiding this comment.
we haven't being following this so far, personally i didn't cause it was just you and me.
| We use flake8 to check some PEP8 errors/warnings. This will be checked with | ||
| ``tox`` and Travis. | ||
|
|
||
| Simple Bandwidth Scanner is moving towards 100% test coverage. Please help us |
There was a problem hiding this comment.
We're at 68% currently, and i think we were already when you wrote that.
I'd rather say that any new feature should come with the test
| ================= | ||
|
|
||
| **Strongly prefer Unix timestamps** over ``datetime`` structures and always | ||
| **Strongly prefer Unix timestamps or datetime objects in UTC** and always |
There was a problem hiding this comment.
and that means using pytz, what has been only needed when converting from unix timestamp to datetime, but not the other way around, because there was not functionality in datetime for that.
However, working on UTC when possible, it's right.
juga0
left a comment
juga0
left a comment
There was a problem hiding this comment.
Adding single comments, but i'd just reuse one CONTRIBUTING file from the many great ones in many great projects and say where it comes from.
|
I think this text is fine, though could be more detailed. |
2 participants
Closes #208