EN-12514: github community standards #4319
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| ## Use development branch | ||
|
|
||
| External contributions should happen against the `development` branch. |
.github/CONTRIBUTING.md
Outdated
| ## Use linter | ||
|
|
||
| Make sure the code is well-formatted and aligned before opening a PR. Some linters such `gofmt` can be used, or code style | ||
| can be proactively checked while written code when using an IDE. |
There was a problem hiding this comment.
writing? + comma before when
.github/CONTRIBUTING.md
Outdated
| ## Writing tests | ||
|
|
||
| In case of adding a new feature, make sure you follow these guidelines: | ||
| - Attempt to write some unit tests which verify the component in isolation and mock the external components around it |
There was a problem hiding this comment.
start all list entries with lower case, end them with ; except the last one which ends with .
Valid for all lists
.github/CONTRIBUTING.md
Outdated
|
|
||
| In case of adding a new feature, make sure you follow these guidelines: | ||
| - Attempt to write some unit tests which verify the component in isolation and mock the external components around it | ||
| - If the component relies on many other external components which are difficult to mock, write integrations tests for it |
There was a problem hiding this comment.
Please add:
Although, this usually means that further decoupling using interfaces is required. Also, the component might needed to be split in smaller, specialized sub-components.
| - Try to first write the unit test that exposes the bug, then fix it and have a test that will make sure the specific situation will always be tested in the future | ||
|
|
||
| In both cases, make sure that the new code has a good code coverage (ideally, 100% of the new lines are covered) and also try to cover edge-cases. | ||
|
|
There was a problem hiding this comment.
I think we should add:
Please take care of the severity of the bug. Pushing directly a fix of an undiscovered, critical bug or a bug that can be exploited in such a way that could create damage to the blockchain, loss of funds, state alteration beyond repair, and so on, can be exploited before the team can take any action. Please contact the team on private channels before pushing the fix if you have any doubts.
| description: By submitting this issue, you agree to follow our [Code of Conduct](https://github.com/ElrondNetwork/elrond-go/blob/main/.github/CODE_OF_CONDUCT.md) | ||
| options: | ||
| - label: I agree to follow this project's Code of Conduct | ||
| required: true |
There was a problem hiding this comment.
Let's also add the version. Something like: v1.3.35.0-0-gfc78258fe/go1.17.6/linux-amd64 and the Host: DO, AWS, Contabo, no VPS (if you run your validator on a physical machine
raduchis
reviewed
Jul 22, 2022
.github/CONTRIBUTING.md
Outdated
| ## Writing tests | ||
|
|
||
| In case of adding a new feature, make sure you follow these guidelines: | ||
| - Attempt to write some unit tests which verify the component in isolation and mock the external components around it |
There was a problem hiding this comment.
Write unit tests which.... remove attempt and some
also two lines down
.github/CONTRIBUTING.md
Outdated
| In case of adding a new feature, make sure you follow these guidelines: | ||
| - Attempt to write some unit tests which verify the component in isolation and mock the external components around it | ||
| - If the component relies on many other external components which are difficult to mock, write integrations tests for it | ||
| - Try to cover both expected functionality but also verify that the code fails in expected ways |
There was a problem hiding this comment.
Remove try to: Cover both expected functionality...
iulianpascalau
approved these changes
Jul 22, 2022
raduchis
approved these changes
Jul 22, 2022
tanghel
approved these changes
Jul 22, 2022
miiu96
approved these changes
Jul 22, 2022
Codecov Report
@@ Coverage Diff @@
## master #4319 +/- ##
=======================================
Coverage 70.90% 70.90%
=======================================
Files 618 618
Lines 82471 82471
=======================================
+ Hits 58478 58480 +2
+ Misses 19765 19764 -1
+ Partials 4228 4227 -1
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
added code of conduct, PR template, contributing guide, and also refactored issue templates