-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Updates to markdown documentation #684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and elsewhere, OpenZeppelin should be a single word.
CONTRIBUTING.md
Outdated
|
||
### D4 - Check preconditions and post-conditions | ||
*IMPORTANT* Please use `rebase` instead of `merge`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.