Updates to markdown documentation #684
Conversation
CONTRIBUTING.md
Outdated
|
|
||
| ### D1 - Simple and Modular | ||
| Simpler code means easier audits, and better understanding of what each component does. We look for small files, small contracts, and small functions. If you can separate a contract into two independent functionalities you should probably do it. | ||
| ## Branching |
There was a problem hiding this comment.
I'd remove this branching section, and simply add a note in the next section to create the pull request against developmnet instead of master.
CONTRIBUTING.md
Outdated
| 5) Maintainers will review your code and possibly ask for changes before your code is pulled in to the main repository. We'll check that all tests pass, review the coding style, and check for general code correctness. If everything is OK, we'll merge your pull request and your code will be part of Open Zeppelin. | ||
|
|
||
| Finally go to [github.com/OpenZeppelin/zeppelin-solidity](https://github.com/OpenZeppelin/zeppelin-solidity) in your web browser and issue a new pull request. | ||
| *IMPORTANT* It is not uncommon for almost finished PRs to stay unmerged for a long time, so, considering the effort you put in your code, please pay attention to the maintainer's feedback since its a necessary to keep up with the standards Open Zeppelin attains to. |
There was a problem hiding this comment.
Though I like the warning, I feel like it's too negative. I'd remove this, and just add the "Please pay attention to the maintainer's feedback since its a necessary to keep up with the standards Open Zeppelin attains to" to the previous paragraph.
frangio
left a comment
frangio
left a comment
There was a problem hiding this comment.
Very good improvement. Thanks @ajsantander!
CONTRIBUTING.md
Outdated
| @@ -1,109 +1,62 @@ | |||
| Contributing to Zeppelin | |||
| Contributing to Open Zeppelin | |||
There was a problem hiding this comment.
Here and elsewhere, OpenZeppelin should be a single word.
CONTRIBUTING.md
Outdated
| As a contributor, you are expected to fork this repository, work on your own fork and then submit pull requests. The pull requests will be reviewed and eventually merged into the main repo. See ["Fork-a-Repo"](https://help.github.com/articles/fork-a-repo/) for how this works. | ||
|
|
||
| ### D4 - Check preconditions and post-conditions | ||
| *IMPORTANT* Please use `rebase` instead of `merge`. |
There was a problem hiding this comment.
Add "when updating your branch to include the latest commits", or something similar that explains in what scenario the contributor is expected to rebase.
CONTRIBUTING.md
Outdated
|
|
||
| ## Pull Request Workflow | ||
| ``` | ||
| cd my-zeppelin-solidity-fork |
There was a problem hiding this comment.
Generally the directory willl be called zeppelin-solidity, so maybe we should use that directory name here.
| git checkout -b fix/some-bug | ||
| git checkout -b remove/some-file | ||
| 2) Branch out from `development` into `fix/some-bug-#123`: | ||
| (Postfixing #123 will associate your PR with the issue #123 and make everyone's life easier =D) |
There was a problem hiding this comment.
I'm not sure if this is required. Is it possible that simply the "closes #pr" is enough for the association?
There was a problem hiding this comment.
CONTRIBUTING.md
Outdated
| git remote add zep [email protected]:OpenZeppelin/zeppelin-solidity.git | ||
| git pull --rebase zep master | ||
| ``` | ||
| git add -A |
There was a problem hiding this comment.
I think I'd prefer to discourage git add --all. Not sure though... It's just that explicitly listing the files feels better to me.
…-updates Updates to markdown documentation
4 participants
Fixes #211
Move contribution guidelines out of CONTRIBUTING.md in order to make it more user friendly, and modified it so that it is more straight forward for a newcomer.
Add a link to the wiki in README.md.