-
Notifications
You must be signed in to change notification settings - Fork 411
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
Add Developer's guide and best practices #1075
Conversation
Very cool to see this. I'll let @alpe review and give approval, but great addition |
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.
This looks great. A lot to read for newbies.
I have added some comments where we work differently. Please adjust.
CODING_GUIDELINES.md
Outdated
* think about maintainbility and testability. | ||
* "Depend upon abstractions, [not] concretions". | ||
* Try to limit the number of methods you are exposing. It's easier to expose something later than to hide it. | ||
* Take advantage of `internal` package concept. |
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.
The internal
package was not working well with wasmd as a library. Let's not bring it back
} | ||
``` | ||
|
||
* It would be an improvement to return `return nil, sdkerror.Wrap(err, "lock contract coins")` |
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
* Fork the repo (core developers must create a branch directly in the Wasmd repo), | ||
branch from the HEAD of `main`, make some commits, and submit a PR to `main`. | ||
* For core developers working within the `wasmd` repo, follow branch name conventions to ensure a clear | ||
ownership of branches: `{moniker}/{issue#}-branch-name`. |
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.
do we want to do this branch pattern? I am doing {issue}_name
only
* For core developers working within the `wasmd` repo, follow branch name conventions to ensure a clear | ||
ownership of branches: `{moniker}/{issue#}-branch-name`. | ||
* See [Branching Model](#branching-model-and-release) for more details. | ||
* Be sure to run `make format` before every commit. The easiest way |
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.
👍 this is good
CONTRIBUTING.md
Outdated
|
||
### Testing | ||
|
||
Tests can be executed by running `make test` at the top level of the Cosmos SDK repository. |
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.
Tests can be executed by running `make test` at the top level of the Cosmos SDK repository. | |
Tests can be executed by running `make test` at the top level of the wasmd repository. |
CONTRIBUTING.md
Outdated
1. If you have something to show, **start with a `Draft` PR**. It's good to have early validation of your work and we highly recommend this practice. A Draft PR also indicates to the community that the work is in progress. | ||
Draft PRs also helps the core team provide early feedback and ensure the work is in the right direction. | ||
2. When the code is complete, change your PR from `Draft` to `Ready for Review`. | ||
3. Go through the actions for each checkbox present in the PR template description. The PR actions are automatically provided for each new PR. |
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.
we don't have checkboxes :-)
CONTRIBUTING.md
Outdated
3. Go through the actions for each checkbox present in the PR template description. The PR actions are automatically provided for each new PR. | ||
4. Be sure to include a relevant changelog entry in the `Unreleased` section of `CHANGELOG.md` (see file for log format). The entry should be on top of all others changes in the section. | ||
|
||
PRs should have a category prefix that is based on the type of changes being made (for example, `fix`, `feat`, |
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.
PR names should start uppercase . Let's not have prefixes. This can look odd in the history
CONTRIBUTING.md
Outdated
PRs should have a category prefix that is based on the type of changes being made (for example, `fix`, `feat`, | ||
`refactor`, `docs`, and so on). The *type* must be included in the PR title as a prefix (for example, | ||
`fix: <description>`). This convention ensures that all changes that are committed to the base branch follow the | ||
[Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification. |
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.
maybe we can add this later when needed. A lot to read for a new contributor
CONTRIBUTING.md
Outdated
[Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification. | ||
Additionally, each PR should only address a single issue. | ||
|
||
Pull requests are merged automatically using [`A:automerge` action](https://mergify.io/features/auto-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.
This is not used, yet
|
||
To generate the protobuf stubs, you can run `make proto-gen`. | ||
|
||
We also added the `make proto-all` command to run all the above commands sequentially. |
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.
👍 my favourite
Hey.. in the name of "Developer's guide and best practices", can we please have a What I mean is a list of steps that gets a new developer up and running with an instance of I'd write it and suggest in the PR, but as of right now I don't know the correct steps. A bit of a guess, but something like:
|
I see scripts at https://github.com/CosmWasm/wasmd/tree/main/contrib/local - maybe adapt these into English instructions with some explanation of what's going on (and reduce to bare minimum, i.e. no need to transfer tokens or deploy contracts to get up and running, that's kinda after everything is working) |
putting that together, my notes, in case it helps: after our chain-id is going to be adding two users: tester1 and validator1
now |
I agree with the general gist of these comments. That can be a new PR, but should be part of or clearly linked from any such dev setup doc. A lot of this should be in an overhauled docs site, and definitely info on connecting to chains, setting up Go, etc. But it would be good to have the notes here. Especially things like compiling on MacOS (the ledger warnings/errors), arm64 compatibility, etc. |
Documentation is an epic were you can always add more or improve it. I think it is important to get something out that we use to iterate and improve. I have create #1081 to follow up on your request @dakom . Thanks a lot for bringing this up! I would be happy if you can help us building better docs. |
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Remove exposed functions * Add tests * Add Developer's guide and best practices (#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <[email protected]> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <[email protected]> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <[email protected]> Co-authored-by: Alex Peters <[email protected]> Co-authored-by: Giancarlos Salas <[email protected]>
* Add Developer's guide and best practices * Fix comments
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <[email protected]> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <[email protected]> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <[email protected]> Co-authored-by: Alex Peters <[email protected]> Co-authored-by: Giancarlos Salas <[email protected]>
* Add Developer's guide and best practices * Fix comments
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <[email protected]> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <[email protected]> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <[email protected]> Co-authored-by: Alex Peters <[email protected]> Co-authored-by: Giancarlos Salas <[email protected]>
* Add Developer's guide and best practices * Fix comments
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <[email protected]> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <[email protected]> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <[email protected]> Co-authored-by: Alex Peters <[email protected]> Co-authored-by: Giancarlos Salas <[email protected]>
Resolves #654