Skip to content

Commit

Permalink
style: lint go and markdown (backport #10060) (#10473)
Browse files Browse the repository at this point in the history
* style: lint go and markdown (#10060)

## Description

+ fixing `x/bank/migrations/v44.migrateDenomMetadata` - we could potentially put a wrong data in a new key if the old keys have variable length.
+ linting the code

Putting in the same PR because i found the issue when running a linter.

Depends on: #10112

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit 479485f)

# Conflicts:
#	CODING_GUIDELINES.md
#	CONTRIBUTING.md
#	STABLE_RELEASES.md
#	contrib/rosetta/README.md
#	cosmovisor/README.md
#	crypto/keyring/keyring.go
#	db/README.md
#	docs/404.md
#	docs/DOCS_README.md
#	docs/architecture/adr-038-state-listening.md
#	docs/architecture/adr-040-storage-and-smt-state-commitments.md
#	docs/architecture/adr-043-nft-module.md
#	docs/architecture/adr-044-protobuf-updates-guidelines.md
#	docs/architecture/adr-046-module-params.md
#	docs/migrations/pre-upgrade.md
#	docs/migrations/rest.md
#	docs/ru/README.md
#	docs/run-node/rosetta.md
#	docs/run-node/run-node.md
#	docs/run-node/run-testnet.md
#	go.mod
#	scripts/module-tests.sh
#	snapshots/README.md
#	store/streaming/README.md
#	store/streaming/file/README.md
#	store/v2/flat/store.go
#	store/v2/smt/store.go
#	x/auth/ante/sigverify.go
#	x/auth/middleware/basic.go
#	x/auth/spec/01_concepts.md
#	x/auth/spec/05_vesting.md
#	x/auth/spec/07_client.md
#	x/authz/spec/05_client.md
#	x/bank/spec/README.md
#	x/crisis/spec/05_client.md
#	x/distribution/spec/README.md
#	x/epoching/keeper/keeper.go
#	x/epoching/spec/03_to_improve.md
#	x/evidence/spec/07_client.md
#	x/feegrant/spec/README.md
#	x/gov/spec/01_concepts.md
#	x/gov/spec/07_client.md
#	x/group/internal/orm/spec/01_table.md
#	x/mint/spec/06_client.md
#	x/slashing/spec/09_client.md
#	x/slashing/spec/README.md
#	x/staking/spec/09_client.md
#	x/upgrade/spec/04_client.md

* fix conflicts

* remove unnecessary files

Co-authored-by: Robert Zaremba <[email protected]>
  • Loading branch information
mergify[bot] and robert-zaremba authored Nov 11, 2021
1 parent 0ac1f6d commit 8a9589a
Show file tree
Hide file tree
Showing 38 changed files with 5,745 additions and 374 deletions.
89 changes: 89 additions & 0 deletions CODING_GUIDELINES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Coding Guidelines

This document is an extension to [CONTRIBUTING](./CONTRIBUTING.md) and provides more details about the coding guidelines and requirements.

## API & Design

+ Code must be well structured:
+ packages must have a limited responsibility (different concerns can go to different packages),
+ types must be easy to compose,
+ 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.
+ Follow agreed-upon design patterns and naming conventions.
+ publicly-exposed functions are named logically, have forward-thinking arguments and return types.
+ Avoid global variables and global configurators.
+ Favor composable and extensible designs.
+ Minimize code duplication.
+ Limit third-party dependencies.

Performance:

+ Avoid unnecessary operations or memory allocations.

Security:

+ Pay proper attention to exploits involving:
+ gas usage
+ transaction verification and signatures
+ malleability
+ code must be always deterministic
+ Thread safety. If some functionality is not thread-safe, or uses something that is not thread-safe, then clearly indicate the risk on each level.

## Testing

Make sure your code is well tested:

+ Provide unit tests for every unit of your code if possible. Unit tests are expected to comprise 70%-80% of your tests.
+ Describe the test scenarios you are implementing for integration tests.
+ Create integration tests for queries and msgs.
+ Use both test cases and property / fuzzy testing. We use the [rapid](pgregory.net/rapid) Go library for property-based and fuzzy testing.
+ Do not decrease code test coverage. Explain in a PR if test coverage is decreased.

We expect tests to use `require` or `assert` rather than `t.Skip` or `t.Fail`,
unless there is a reason to do otherwise.
When testing a function under a variety of different inputs, we prefer to use
[table driven tests](https://github.com/golang/go/wiki/TableDrivenTests).
Table driven test error messages should follow the following format
`<desc>, tc #<index>, i #<index>`.
`<desc>` is an optional short description of whats failing, `tc` is the
index within the test case table that is failing, and `i` is when there
is a loop, exactly which iteration of the loop failed.
The idea is you should be able to see the
error message and figure out exactly what failed.
Here is an example check:

```go
<some table>
for tcIndex, tc := range cases {
<some code>
resp, err := doSomething()
require.NoError(err)
require.Equal(t, tc.expected, resp, "should correctly perform X")
```
## Quality Assurance
We are forming a QA team that will support the core Cosmos SDK team and collaborators by:
- Improving the Cosmos SDK QA Processes
- Improving automation in QA and testing
- Defining high-quality metrics
- Maintaining and improving testing frameworks (unit tests, integration tests, and functional tests)
- Defining test scenarios.
- Verifying user experience and defining a high quality.
- We want to have **acceptance tests**! Document and list acceptance lists that are implemented and identify acceptance tests that are still missing.
- Acceptance tests should be specified in `acceptance-tests` directory as Markdown files.
- Supporting other teams with testing frameworks, automation, and User Experience testing.
- Testing chain upgrades for every new breaking change.
- Defining automated tests that assure data integrity after an update.
Desired outcomes:
- QA team works with Development Team.
- QA is happening in parallel with Core Cosmos SDK development.
- Releases are more predictable.
- QA reports. Goal is to guide with new tasks and be one of the QA measures.
As a developer, you must help the QA team by providing instructions for User Experience (UX) and functional testing.
318 changes: 113 additions & 205 deletions CONTRIBUTING.md

Large diffs are not rendered by default.

143 changes: 0 additions & 143 deletions STABLE_RELEASES.md

This file was deleted.

6 changes: 5 additions & 1 deletion contrib/rosetta/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ Contains the required files to set up rosetta cli and make it work against its w

## node

Contains the files for a deterministic network, with fixed keys and some actions on there, to test parsing of msgs and historical balances.
Contains the files for a deterministic network, with fixed keys and some actions on there, to test parsing of msgs and historical balances. This image is used to run a simapp node and to run the rosetta server.

## Rosetta-cli

The docker image for ./rosetta-cli/Dockerfile is on [docker hub](https://hub.docker.com/r/tendermintdev/rosetta-cli). Whenever rosetta-cli releases a new version, rosetta-cli/Dockerfile should be updated to reflect the new version and pushed to docker hub.

## Notes

Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/multisig/amino.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func tmToProto(tmPk tmMultisig) (*LegacyAminoPubKey, error) {
}

// MarshalAminoJSON overrides amino JSON unmarshaling.
func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { //nolint:golint
func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { //nolint:revive
return protoToTm(&m)
}

Expand Down
1 change: 1 addition & 0 deletions crypto/keys/secp256k1/secp256k1_nocgo.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !libsecp256k1
// +build !libsecp256k1

package secp256k1
Expand Down
2 changes: 2 additions & 0 deletions crypto/ledger/ledger_notavail.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//go:build !cgo || !ledger
// +build !cgo !ledger

// test_ledger_mock

package ledger
Expand Down
47 changes: 47 additions & 0 deletions docs/404.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!--
permalink: /404.html
layout: homepage
title: 404 Lost in Space!
description: You've been lost in space
sections:
- title: Introduction
desc: High-level overview of the Cosmos SDK.
url: /intro/overview.html
icon: introduction
- title: Basics
desc: Anatomy of a blockchain, transaction lifecycle, accounts and more.
icon: basics
url: /basics/app-anatomy.html
- title: Core Concepts
desc: Read about the core concepts like baseapp, the store, or the server.
icon: core
url: /core/baseapp.html
- title: Building Modules
desc: Discover how to build modules for the Cosmos SDK.
icon: modules
url: /building-modules/intro.html
- title: Running a Node
desc: Running and interacting with nodes using the CLI and API.
icon: interfaces
url: /run-node/
- title: Modules
desc: Explore existing modules to build your application with.
icon: specifications
url: /modules/
stack:
- title: Cosmos Hub
desc: The first of thousands of interconnected blockchains on the Cosmos Network.
color: "#BA3FD9"
label: hub
url: http://hub.cosmos.network
- title: Tendermint Core
desc: The leading BFT engine for building blockchains, powering Cosmos SDK.
color: "#00BB00"
label: core
url: http://docs.tendermint.com
footer:
newsletter: false
aside: false
-->

# 404 - Lost in space, this is just an empty void
24 changes: 16 additions & 8 deletions docs/DOCS_README.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
# Updating the docs

If you want to open a PR on the Cosmos SDK to update the documentation, please follow the guidelines in the [`CONTRIBUTING.md`](https://github.com/cosmos/cosmos-sdk/tree/master/CONTRIBUTING.md#updating-documentation)

## Translating

- Docs translations live in a `docs/country-code/` folder, where `country-code` stands for the country code of the language used (`cn` for Chinese, `kr` for Korea, `fr` for France, ...).
- Always translate content living on `master`.
- Only content under `/docs/intro/`, `/docs/basics/`, `/docs/core/`, `/docs/building-modules/` and `docs/run-node/` needs to be translated, as well as `docs/README.md`. It is also nice (but not mandatory) to translate `/docs/spec/`.
- Specify the release/tag of the translation in the README of your translation folder. Update the release/tag each time you update the translation.
If you want to open a PR in Cosmos SDK to update the documentation, please follow the guidelines in [`CONTRIBUTING.md`](https://github.com/cosmos/cosmos-sdk/tree/master/CONTRIBUTING.md#updating-documentation).

## Internationalization

- Translations for documentation live in a `docs/<locale>/` folder, where `<locale>` is the language code for a specific language. For example, `zh` for Chinese, `ko` for Korean, `ru` for Russian, etc.
- Each `docs/<locale>/` folder must follow the same folder structure within `docs/`, but only content in the following folders needs to be translated and included in the respective `docs/<locale>/` folder:
- `docs/basics/`
- `docs/building-modules/`
- `docs/core/`
- `docs/ibc/`
- `docs/intro/`
- `docs/migrations/`
- `docs/run-node/`
- Each `docs/<locale>/` folder must also have a `README.md` that includes a translated version of both the layout and content within the root-level [`README.md`](https://github.com/cosmos/cosmos-sdk/tree/master/docs/README.md). The layout defined in the `README.md` is used to build the homepage.
- Always translate content living on `master` unless you are revising documentation for a specific release. Translated documentation like the root-level documentation is semantically versioned.
- For additional configuration options, please see [VuePress Internationalization](https://vuepress.vuejs.org/guide/i18n.html).

## Docs Build Workflow

Expand Down
4 changes: 0 additions & 4 deletions docs/architecture/adr-010-modular-antehandler.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,6 @@ Cons:
1. Decorator pattern may have a deeply nested structure that is hard to understand, this is mitigated by having the decorator order explicitly listed in the `ChainAnteDecorators` function.
2. Does not make use of the ModuleManager design. Since this is already being used for BeginBlocker/EndBlocker, this proposal seems unaligned with that design pattern.

## Status

> Accepted Simple Decorators approach
## Consequences

Since pros and cons are written for each approach, it is omitted from this section
Expand Down
4 changes: 0 additions & 4 deletions docs/architecture/adr-022-custom-panic-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,6 @@ func (app *BaseApp) AddRunTxRecoveryHandler(handlers ...RecoveryHandler) {

This method would prepend handlers to an existing chain.

## Status

Proposed

## Consequences

### Positive
Expand Down
Loading

0 comments on commit 8a9589a

Please sign in to comment.