Skip to content

op-node/batcher/proposer: Standardize goimports#3403

Merged
mergify[bot] merged 5 commits intodevelopfrom
jg/imports
Sep 10, 2022
Merged

op-node/batcher/proposer: Standardize goimports#3403
mergify[bot] merged 5 commits intodevelopfrom
jg/imports

Conversation

@trianglesphere
Copy link
Contributor

Description

This standardizes go imports across the three go codebases. This removes
empty lines between import sections & relies on goimports to sort the rest.
Imports from the ethereum-optimism and go-ethereum repositories are
treated as local imports.

Additional Information

I used the following script to implement this change:

#!/bin/bash                                                                      
                                                                                 
# remove all blank lines in go 'imports' statements,                             
# then sort with goimports                                                       
# Usage over a codebase: find . -name "*.go" -exec ./goimports2.sh {} \;
                                                                                 
if [ $# != 1 ] ; then                                                            
  echo "usage: $0 <filename>"                                                    
  exit 1                                                                         
fi                                                                               

# Remove empty lines inside the import statement                                                                                 
sed -i '' '/^import/,/^)/ {/^$/d;}' $1                                                                             

#goimports -w $1
#goimports -local "github.com/ethereum-optimism/optimism" -w $1
goimports -local "github.com/ethereum/go-ethereum","github.com/ethereum-optimism/optimism" -w $1

@changeset-bot
Copy link

changeset-bot bot commented Sep 10, 2022

⚠️ No Changeset found

Latest commit: cd28ab8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@trianglesphere trianglesphere requested review from mslipper, protolambda and tynes and removed request for protolambda September 10, 2022 17:17
@mergify
Copy link
Contributor

mergify bot commented Sep 10, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@protolambda
Copy link
Contributor

LGTM, can you put the script in CI? Like the other diff checks, we can run it, and then check if there's a diff to confirm the contents are formatted correctly.

@trianglesphere
Copy link
Contributor Author

trianglesphere commented Sep 10, 2022

can you put the script in CI? Like the other diff checks, we can run it, and then check if there's a diff to confirm the contents are formatted correctly.

Not reliably.

There's two parts of this: killing the newlines between the different import sections & doing goimports. We already have goimports, but this mainly does does the killing newlines. Unfortunately the sed script is not reliable enough when there is a single import line to run.

@mergify
Copy link
Contributor

mergify bot commented Sep 10, 2022

Hey @trianglesphere! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added conflict and removed conflict labels Sep 10, 2022
@mergify
Copy link
Contributor

mergify bot commented Sep 10, 2022

Hey @trianglesphere! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Sep 10, 2022
@mergify
Copy link
Contributor

mergify bot commented Sep 10, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify mergify bot merged commit 25b088f into develop Sep 10, 2022
@mergify mergify bot deleted the jg/imports branch September 10, 2022 19:56
@mergify
Copy link
Contributor

mergify bot commented Sep 10, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot removed the on-merge-train label Sep 10, 2022
maurelian pushed a commit that referenced this pull request Sep 15, 2022
* op-node: goimports

* op-batcher: goimports

* op-proposer: goimports

* Fix deleted line due to overeager sed

* fix imports

Co-authored-by: protolambda <proto@protolambda.com>
sam-goldman pushed a commit that referenced this pull request Sep 15, 2022
* op-node: goimports

* op-batcher: goimports

* op-proposer: goimports

* Fix deleted line due to overeager sed

* fix imports

Co-authored-by: protolambda <proto@protolambda.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants