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

convert .format to f-strings #144

Merged
merged 2 commits into from Oct 11, 2023
Merged

Conversation

pacrob
Copy link
Contributor

@pacrob pacrob commented Oct 10, 2023

What was wrong?

  • Lib still used old-style .format strings.
  • mdformat markdown linter did not have the github-flavored extension, so messed with checkboxes

How was it fixed?

  • Converted to f-strings.
  • Added github flavoring to mdformat
  • Bumped pre-commit hooks version to latest

Todo:

  • Clean up commit history
  • Add entry to the release notes
  • Add or update documentation related to these changes

Cute Animal Picture

image

@pacrob pacrob force-pushed the f-strings-please branch 2 times, most recently from 6d1e3c0 to b509e15 Compare October 11, 2023 14:25
@pacrob pacrob requested review from reedsa and fselmo October 11, 2023 14:27
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

Just one missing f in an f-string

@@ -25,7 +25,7 @@ def serialize(self, obj):
raise SerializationError("Can only serialize integers", obj)
if self.length is not None and obj >= 256**self.length:
raise SerializationError(
"Integer too large (does not fit in {} " "bytes)".format(self.length),
"Integer too large (does not fit in {self.length} bytes)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing f here. This will print "self.length" not the value of self.length

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thank you!

@pacrob pacrob merged commit 8a6d337 into ethereum:master Oct 11, 2023
19 checks passed
@pacrob pacrob deleted the f-strings-please branch October 11, 2023 20:28
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.

None yet

3 participants