Update contributing.md

[shaistha24]

Description

updated the content of the contributing.md for better guidance.

Affected Dependencies

none

How has this been tested?

none needed

Checklist

• I have followed the Contribution Guidelines and Code of Conduct
• I have commented my code following the OpenMined Styleguide
• I have labeled this PR with the relevant Type labels
• My changes are covered by tests

[shaistha24]

Sorry i did change it on my end but there might be a commit issue or something after making the changes... I will make changes asap!

[shaistha24]

Done
Made changes to the readme as per #187 too.

[chinmayshah99]

Yeah, that should be done. Otherwise one always has to push yet another commit saying style fix 😂. Regards Chinmay Shah

…

On Fri, Jun 19, 2020, 10:54 Saksham ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contributing.md <#186 (comment)>: > +- Description - summary of the change, the motivation, and any additionl context that will help others understand your PR +- Affected Dependencies - List any dependencies that are required for this change. +- **How has this been tested? (Important)** + - Describe the tests that you ran to verify your changes. + - Provide instructions so we can reproduce. + - List any relevant details for your test configuration. +- Checklist - Following the code of conduct and basic guidelines. + + +### Check CI and Wait for Reviews + +After each commit GitHub Actions will check your new code against the formatting guidelines and execute the tests to check if the test coverage is high enough. + +We will only merge PRs that pass the GitHub Actions checks. + +If your check fails, don't worry, you will still be able to make changes and make your code pass the checks. I think we should also add a note to run black or clang-format locally before they push so that they can fix those style errors. @chinmayshah99 <https://github.com/chinmayshah99> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#186 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACTTOFPW3SFWQH2HE7GTVOLRXLZAXANCNFSM4N722GIQ> .

[codeboy5]

Yeah, that should be done.
Otherwise one always has to push yet another commit saying style fix 😂.

Regards

Chinmay Shah

On Fri, Jun 19, 2020, 10:54 Saksham notifications@github.com wrote:

@codeboy5 commented on this pull request.

In contributing.md
#186 (comment):

+- Description - summary of the change, the motivation, and any additionl context that will help others understand your PR
+- Affected Dependencies - List any dependencies that are required for this change.
+- How has this been tested? (Important)

• • Describe the tests that you ran to verify your changes.
• • Provide instructions so we can reproduce.
• • List any relevant details for your test configuration.
    +- Checklist - Following the code of conduct and basic guidelines.

+### Check CI and Wait for Reviews
+
+After each commit GitHub Actions will check your new code against the formatting guidelines and execute the tests to check if the test coverage is high enough.
+
+We will only merge PRs that pass the GitHub Actions checks.
+
+If your check fails, don't worry, you will still be able to make changes and make your code pass the checks.

I think we should also add a note to run black or clang-format locally
before they push so that they can fix those style errors. @chinmayshah99
https://github.com/chinmayshah99 ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#186 (comment), or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ACTTOFPW3SFWQH2HE7GTVOLRXLZAXANCNFSM4N722GIQ
.

Yeah, took me a while to get used to.
We can also add a pre commit hook which will kind of automate this.

https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/

[chinmayshah99]

Yeah, took me a while to get used to.
We can also add a pre commit hook which will kind of automate this.

https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/

This looks super clean. Will this do clang-format too?

[chinmayshah99]

@shaistha24 are the requested changes already made?

[jeamick]

LGTM !
Black/clang format should be run before the commit and check on the github action side I think.

[chinmayshah99]

We already have checks on github actions, called Style checks :)

…

On Mon, 29 Jun 2020 at 02:44, jeamick ***@***.***> wrote: ***@***.**** approved this pull request. LGTM ! Black/clang format should be run before the commit and check on the github action side I think. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#186 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACTTOFK5R3I4KBWJEZLFI4TRY6XC7ANCNFSM4N722GIQ> .

-- Regards Chinmay Shah

[Harkirat155]

Review complete for the commit.
I've made some changes, review them and add the updates to the work otherwise it all seems good.

[shaistha24]

@chinmayshah99 could u pls approve it!

[jeamick]

LGTM in terms of typo errors