Skip to content
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

feat:add autopep8 format #31

Merged
merged 2 commits into from
Sep 1, 2023
Merged

feat:add autopep8 format #31

merged 2 commits into from
Sep 1, 2023

Conversation

GrapeBaBa
Copy link
Contributor

fix #28

@GrapeBaBa GrapeBaBa requested a review from norswap August 27, 2023 13:32
Copy link
Member

@norswap norswap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already approving, but could you check the line with the weird line length + add a version check?

Feel free to rebase immediately when its done!

pip3 install --upgrade autopep8
else
echo "autopep8 is installed"
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably also want version checks here to avoid random breakage in CI.

l1.py Outdated
@@ -174,7 +173,8 @@ def start_devnet_l1_node(paths: OPPaths):
except Exception:
running = False
if running:
raise Exception("Couldn't start L1 node: server already running at localhost:8545")
raise Exception(
"Couldn't start L1 node: server already running at localhost:8545")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird, this is less than 100 characters long.


format: ## Run format
@autopep8 --in-place --recursive . --exclude optimism,op-geth,venv --max-line-length 120
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually go for 100 as line length. Do you have strong opinions on 120?

@norswap
Copy link
Member

norswap commented Sep 1, 2023

Made the fixes, merging

@norswap norswap merged commit 9a255a1 into 0xFableOrg:master Sep 1, 2023
2 checks passed
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.

Need an unique code formatter
2 participants