Skip to content

Commit

Permalink
linting: add gofumpt (#105)
Browse files Browse the repository at this point in the history
* replace goimports with gofumpt

* add vscode settings

* golangci-lint run superchain/... validation/... --fix

* enable goimports too

* gitignore .vscode and document in conrtrib guide

* Update CONTRIBUTING.md

Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

* git rm -rf .vscode/

---------

Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
  • Loading branch information
geoknee and mds1 authored Mar 7, 2024
1 parent 46f8c2f commit 2164106
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 26 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
tag: '1.21'
steps:
- checkout
- run: golangci-lint run superchain validation
- run: golangci-lint run superchain/... validation/...
golang-modules-tidy:
executor:
name: go/default # is based on cimg/go
Expand Down
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,7 @@ docs/


# Go workspaces checksum file
go.work.sum
go.work.sum

# editor specific config
.vscode
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ run:
linters:
# also see https://golangci-lint.run/usage/linters/#enabled-by-default
enable:
- gofumpt
- goimports
- bodyclose
- asciicheck
Expand Down
20 changes: 17 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ EOF
Superchain-wide configuration, like the `ProtocolVersions` contract address, should be configured here when available.

### Approved contract versions
Each superchain target should have a `semver.yaml` file in the same directory declaring the approved contract semantic versions for that superchain, e.g:
Each superchain target should have a `semver.yaml` file in the same directory declaring the approved contract semantic versions for that superchain, e.g:
```yaml
l1_cross_domain_messenger: 1.4.0
l1_erc721_bridge: 1.0.0
Expand All @@ -41,7 +41,7 @@ system_config: 1.3.0

# superchain-wide contracts
protocol_versions: 1.0.0
superchain_config:
superchain_config:
```
### `implementations`
Expand Down Expand Up @@ -168,4 +168,18 @@ go run ./op-chain-ops/cmd/registry-data \
--l2-genesis=$GENESIS_CONFIG \
--bytecodes-dir=$SUPERCHAIN_REPO/superchain/extra/bytecodes \
--output=$SUPERCHAIN_REPO/superchain/extra/genesis/$SUPERCHAIN_TARGET/$CHAIN_NAME.json.gz
```
```

## Setting up your editor for formatting and linting
If you use VSCode, you can place the following in a `settings.json` file in the gitignored `.vscode` directory:

```json
{
"go.formatTool": "gofumpt",
"go.lintTool": "golangci-lint",
"go.lintOnSave": "workspace",
"gopls": {
"formatting.gofumpt": true,
},
}
```
2 changes: 0 additions & 2 deletions superchain/superchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,6 @@ func unMarshalSuperchainConfig(data []byte, s *SuperchainConfig) error {
}

return yaml.Unmarshal(data, temp)

}

type Superchain struct {
Expand Down Expand Up @@ -529,7 +528,6 @@ func isConfigFile(c fs.DirEntry) bool {
strings.HasSuffix(c.Name(), ".yaml") &&
c.Name() != "superchain.yaml" &&
c.Name() != "semver.yaml")

}

func init() {
Expand Down
14 changes: 7 additions & 7 deletions superchain/superchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestVersionFor(t *testing.T) {
_, err = cl.VersionFor("Garbage")
require.Error(t, err)
}

func TestChainIds(t *testing.T) {
chainIDs := map[uint64]bool{}

Expand Down Expand Up @@ -159,7 +160,6 @@ func TestContractVersionsCheck(t *testing.T) {
t.Fatal(err)
}
}

}

// TestContractVersionsResolve will test that the high lever interface used works.
Expand Down Expand Up @@ -432,16 +432,17 @@ fjord_time:
}

func TestHardForkOverridesAndDefaults(t *testing.T) {

defaultCanyonTime := uint64(3)
defaultSuperchainConfig := SuperchainConfig{
hardForkDefaults: HardForkConfiguration{
CanyonTime: &defaultCanyonTime,
}}
},
}
nilDefaultSuperchainConfig := SuperchainConfig{
hardForkDefaults: HardForkConfiguration{
CanyonTime: nil,
}}
},
}

overridenCanyonTime := uint64Ptr(uint64(8))
override := []byte(`canyon_time: 8`)
Expand Down Expand Up @@ -478,16 +479,15 @@ func TestHardForkOverridesAndDefaults(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) { executeTestCase(t, tt) })
}

}

func TestHardForkOverridesAndDefaults2(t *testing.T) {

defaultSuperchainConfig := SuperchainConfig{
hardForkDefaults: HardForkConfiguration{
CanyonTime: uint64Ptr(0),
DeltaTime: uint64Ptr(1),
}}
},
}

c := ChainConfig{}

Expand Down
4 changes: 0 additions & 4 deletions validation/gpo-params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func TestGasPriceOracleParams(t *testing.T) {
gasPriceOraclAddr := predeploys.GasPriceOracleAddr

checkPreEcotoneResourceConfig := func(t *testing.T, chain *ChainConfig, client *ethclient.Client) {

desiredParamsOuter, ok := GasPriceOracleParams[chain.Superchain]

if !ok {
Expand All @@ -56,11 +55,9 @@ func TestGasPriceOracleParams(t *testing.T) {
"scalar parameter %d out of bounds %d", actualParams.Scalar, desiredParams.Scalar.Bounds)

t.Logf("gas price oracle params are acceptable")

}

checkEcotoneResourceConfig := func(t *testing.T, chain *ChainConfig, client *ethclient.Client) {

desiredParamsOuter, ok := GasPriceOracleParams[chain.Superchain]

if !ok {
Expand All @@ -83,7 +80,6 @@ func TestGasPriceOracleParams(t *testing.T) {
"baseFeeScalar parameter %d out of bounds %d", actualParams.BaseFeeScalar, desiredParams.BaseFeeScalar.Bounds)

t.Logf("gas price oracle params are acceptable")

}

checkResourceConfig := func(t *testing.T, chain *ChainConfig, client *ethclient.Client) {
Expand Down
3 changes: 0 additions & 3 deletions validation/l2oo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
)

func TestL2OOParams(t *testing.T) {

isExcluded := map[uint64]bool{
10: true, // mainnet/op (old version of L2OutputOracle, no submissionInterval method)
291: true, // mainnet/orderly (old version of L2OutputOracle, no submissionInterval method)
Expand All @@ -42,7 +41,6 @@ func TestL2OOParams(t *testing.T) {

incorrectMsg := func(name string, want, got *big.Int) string {
return fmt.Sprintf("Incorrect %s, wanted %d got %d", name, want, got)

}

requireEqualParams := func(t *testing.T, desired, actual L2OOParams) {
Expand Down Expand Up @@ -90,7 +88,6 @@ func TestL2OOParams(t *testing.T) {
requireEqualParams(t, desiredParams, actualParams)

t.Logf("L2OutputOracle config params acceptable")

}

for chainID, chain := range OPChains {
Expand Down
1 change: 0 additions & 1 deletion validation/resource-config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func TestResourceConfig(t *testing.T) {
require.Equal(t, bindings.ResourceMeteringResourceConfig(OPMainnetResourceConfig), actualResourceConfig, "resource config unacceptable")

t.Logf("resource metering acceptable")

}

for chainID, chain := range OPChains {
Expand Down
3 changes: 0 additions & 3 deletions validation/superchain-version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ var isSemverAcceptable = func(desired, actual string) bool {
}

func TestSuperchainWideContractVersions(t *testing.T) {

checkSuperchainTargetSatisfiesSemver := func(t *testing.T, superchain *Superchain) {

rpcEndpoint := superchain.Config.L1.PublicRPC
require.NotEmpty(t, rpcEndpoint)

Expand Down Expand Up @@ -53,7 +51,6 @@ func TestSuperchainWideContractVersions(t *testing.T) {
for superchainName, superchain := range Superchains {
t.Run(superchainName, func(t *testing.T) { checkSuperchainTargetSatisfiesSemver(t, superchain) })
}

}

func TestContractVersions(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion validation/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var isWithinBounds = func(actual uint32, bounds [2]uint32) bool {
panic("bounds are in wrong order")
}
return (actual >= bounds[0] && actual <= bounds[1])

}

func TestAreCloseInts(t *testing.T) {
Expand Down

0 comments on commit 2164106

Please sign in to comment.