Docs/616 contributing #633

ClaudiaComito · 2020-07-16T09:53:51Z

Description

Update to contribution guidelines.

Issue/s resolved: #616

Changes proposed:

Added intro with most important info and link to Mattermost
Few edits in the standard guidelines
Added link to upcoming documentation_howto.rst, this link currently points to the 504-docstring-formatting branch, must be updated once that is merged.

Type of change

Documentation update

Due Diligence

Documentation updated (if needed)
Updated changelog.md under the title "Pending Additions"

Does this change modify the behaviour of other functions? If so, which?

no

Markus-Goetz · 2020-07-16T09:55:33Z

contributing.md

@@ -43,7 +50,8 @@ git pull upstream master
 git checkout -b features/123-boolean-operators
 ```

-* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's docstring standard (explained below).
+* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's [docstring standard](https://github.com/helmholtz-analytics/heat/blob/504-docstring-formatting/doc/source/documentation_howto.rst).


The link to the docstrings branch is somewhat unfortunate. Should have this as a TODO in 504 to let it point to master

Markus-Goetz · 2020-07-16T09:56:08Z

contributing.md

@@ -55,9 +63,9 @@ git push origin features/123-boolean-operators

 * Enter your GitHub username and password (advanced users can remove this step by connecting to GitHub with SSH.

-* Go to GitHub. The new branch will show up with a green Pull Request button. Make sure the title and message are clear, concise, and self- explanatory. Then click the button to submit it.
+* Go to GitHub. The new branch will show up with a green Pull Request button. Make sure the title and message are clear, concise, and self-explanatory. Then click the button to submit it.


Perhaps a statement on rebasing here?

Hmm. Thanks for reminding me (to rebase).

Do we know what we want well enough to write it down in the guidelines? Do we want to ask the contributors to tidy up their commits only keeping the most important ones (git rebase -i master and then, in most cases, force push), or do we want to rebase by default before every PR? We haven't been sticking to any particular rule so far...

They should rebase before the PR. After the PR is live, no more rebasing. Moreover, should we make a general reference with link towards our development process (what is it now btw Github Flow or Gitlab Flow?)

I don't think we have reached an agreement on this. I would leave it all out for now.

I've mentioned rebasing before pushing as we discussed offline.

codecov · 2020-07-16T09:57:07Z

Codecov Report

Merging #633 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #633   +/-   ##
=======================================
  Coverage   97.40%   97.40%           
=======================================
  Files          79       79           
  Lines       16126    16126           
=======================================
  Hits        15708    15708           
  Misses        418      418

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d13e354...090479a. Read the comment docs.

Cdebus

Looks good, just 2-3 minor things

Cdebus · 2020-07-16T14:22:59Z

contributing.md

@@ -1,6 +1,13 @@
 ## Contributing to HeAT

-*These contribution guidelines are inspired by NumPy's contribution guidelines of the SciPy community.*
+Thanks for your interest in contributing to HeAT!


"Thank you"....
And maybe add something: "We appreciate your effort" ?

Cdebus · 2020-07-16T14:24:28Z

contributing.md

@@ -43,7 +50,8 @@ git pull upstream master
 git checkout -b features/123-boolean-operators
 ```

-* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's docstring standard (explained below).
+* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's [docstring standard](https://github.com/helmholtz-analytics/heat/blob/504-docstring-formatting/doc/source/documentation_howto.rst).


Dot missing after (git add and git commit) ... as in end of sentence

Good catch! Done

Cdebus · 2020-07-16T14:26:58Z

contributing.md

@@ -43,7 +50,8 @@ git pull upstream master
 git checkout -b features/123-boolean-operators
 ```

-* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's docstring standard (explained below).
+* Commit locally as you progress (`git add` and `git commit`) Use a properly formatted commit message, write tests that fail before your change and pass afterward, run all the tests locally and in parallel for different process counts (`mpirun -np <PROCESSES>`). Be sure to document any changed behavior in docstrings, keeping to HeAT's [docstring standard](https://github.com/helmholtz-analytics/heat/blob/504-docstring-formatting/doc/source/documentation_howto.rst).


I like the link to the docstrings... can we mention that again further down under "guidelines" ? (line 100 )

Yes good idea, done

ClaudiaComito · 2020-07-17T14:14:45Z

@Markus-Goetz @Cdebus I'm unhappy about the pre-commit information being on README. I'm going to move it to contributing.md and have README point to the contribution guidelines, unless somebody screams.

Markus-Goetz · 2020-07-17T14:26:24Z

Seems reasonable

ClaudiaComito · 2020-07-17T16:26:31Z

Seems reasonable

I think I'm done. See you next month!

ClaudiaComito added the documentation Anything related to documentation label Jul 16, 2020

ClaudiaComito requested review from Markus-Goetz and Cdebus July 16, 2020 09:53

Markus-Goetz reviewed Jul 16, 2020

View reviewed changes

Update contributing.md

2ababcf

ClaudiaComito force-pushed the docs/616-contributing branch from 2d5b561 to 2ababcf Compare July 16, 2020 10:07

Cdebus requested changes Jul 16, 2020

View reviewed changes

ClaudiaComito added 2 commits July 17, 2020 15:59

Add corrections, rebase instructions, TL;DR

99e6193

Update changelog

794705f

ClaudiaComito added 3 commits July 17, 2020 16:48

Link and move pre-commit instructions to contributing.md

a7bd550

Add pre-commit instructions

7843aff

Format numbering

6d27017

Cdebus previously approved these changes Jul 21, 2020

View reviewed changes

Merge branch 'master' into docs/616-contributing

090479a

Cdebus dismissed their stale review via 090479a July 21, 2020 07:47

Cdebus approved these changes Jul 21, 2020

View reviewed changes

Cdebus merged commit 6629499 into master Jul 21, 2020

Cdebus deleted the docs/616-contributing branch July 21, 2020 08:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs/616 contributing #633

Docs/616 contributing #633

ClaudiaComito commented Jul 16, 2020 •

edited

Loading

Markus-Goetz Jul 16, 2020

ClaudiaComito Jul 16, 2020

Markus-Goetz Jul 16, 2020

ClaudiaComito Jul 16, 2020

Markus-Goetz Jul 16, 2020

ClaudiaComito Jul 17, 2020

ClaudiaComito Jul 17, 2020

codecov bot commented Jul 16, 2020 •

edited

Loading

Cdebus left a comment

Cdebus Jul 16, 2020

ClaudiaComito Jul 17, 2020

Cdebus Jul 16, 2020

ClaudiaComito Jul 17, 2020

Cdebus Jul 16, 2020

ClaudiaComito Jul 17, 2020

ClaudiaComito commented Jul 17, 2020

Markus-Goetz commented Jul 17, 2020

ClaudiaComito commented Jul 17, 2020

Docs/616 contributing #633

Docs/616 contributing #633

Conversation

ClaudiaComito commented Jul 16, 2020 • edited Loading

Description

Changes proposed:

Type of change

Due Diligence

Does this change modify the behaviour of other functions? If so, which?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 16, 2020 • edited Loading

Codecov Report

Cdebus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ClaudiaComito commented Jul 17, 2020

Markus-Goetz commented Jul 17, 2020

ClaudiaComito commented Jul 17, 2020

ClaudiaComito commented Jul 16, 2020 •

edited

Loading

codecov bot commented Jul 16, 2020 •

edited

Loading