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

cleanup: f-string formatting #789

Merged
merged 6 commits into from
May 16, 2022
Merged

cleanup: f-string formatting #789

merged 6 commits into from
May 16, 2022

Conversation

zabop
Copy link
Contributor

@zabop zabop commented May 13, 2022

As discussed here, code in this repo appears directly in the documentation. (An example, there are hundreds.) This commit introduces f-strings to increase readability of both code and documentation.

Fixes #785 🦕

@zabop zabop requested review from a team as code owners May 13, 2022 17:42
@zabop zabop requested a review from kurtisvg May 13, 2022 17:42
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/python-storage API. labels May 13, 2022
@unforced unforced added the type: cleanup An internal cleanup or hygiene concern. label May 13, 2022
@unforced unforced changed the title f-string formatting cleanup: f-string formatting May 13, 2022
@unforced unforced added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2022
@zabop
Copy link
Contributor Author

zabop commented May 13, 2022

I am not sure why 2 tests failed, let me know if further input is needed from me.

@unforced
Copy link
Contributor

Yep, for the conventionalcommits message, you just need to amend the commit message to be "cleanup: f-string formatting". I thought editing the title there would do it, but seems not.
For the other one, you can run nox -s blacken lint and it'll lint cleanup all the files.
If you're not able to do either of those, let me know and I can help.

Copy link
Contributor

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

Thank you. Mostly looks correct, except for a few spots.

docs/snippets.py Outdated Show resolved Hide resolved
samples/snippets/notification_polling.py Show resolved Hide resolved
tests/unit/test_blob.py Outdated Show resolved Hide resolved
tests/unit/test_blob.py Outdated Show resolved Hide resolved
@zabop
Copy link
Contributor Author

zabop commented May 13, 2022

@Mariatta, thank you, great catches!
@Breathtender, I get an error with nox:

% nox -s blacken lint
nox > Running session blacken
nox > Session blacken skipped: Python interpreter 3.8 not found.
nox > Running session lint
nox > Session lint skipped: Python interpreter 3.8 not found.
nox > Ran multiple sessions:
nox > * blacken: skipped
nox > * lint: skipped

Which I do not yet know how to solve. My Python interpreter seems to be Python 3.8 (screenshot of pottom right corner of PyCharm):

image

@unforced
Copy link
Contributor

Oh that's strange. I'll check on why that's happening. For now I'll just run the lint myself and add the commit, once you push up your changes to address @Mariatta's comments.

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 13, 2022
@zabop
Copy link
Contributor Author

zabop commented May 13, 2022

@Breathtender, just read your reply now, sorry; in the meantime I figured out what's going on and were able to run nox -s blacken lint. Just pushed the results.

@zabop
Copy link
Contributor Author

zabop commented May 13, 2022

Addressing @Mariatta suggestions now...

zabop and others added 4 commits May 13, 2022 22:57
Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
@zabop
Copy link
Contributor Author

zabop commented May 13, 2022

I think I have addressed the suggestions.

My last commit was the result of a typo & forgetting to run nox -s blacken lint before. Do I have to keep the cleanup: f-string formatting commit messages when I push fixes to an already open PR, or is more freedom allowed here? (To rephrase the question: which commit message is checked by the conventionalcommits test?) I used the remove unnecessary :d specifier commit message to approve @Mariatta suggestions, I hope that's ok.

@unforced
Copy link
Contributor

No, it'll automatically roll all your commits together, and so the first commit message is ultimately the one that matters.

@unforced
Copy link
Contributor

Thanks for making those adjustments! I'll wait for @Mariatta's approval on the changes requested, but this is looking good to me.

Copy link
Contributor

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@Mariatta Mariatta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2022
@unforced unforced merged commit 3664dde into googleapis:main May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/python-storage API. size: l Pull request size is large. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Let's use f-strings instead of .format() to increase readability of documentation
4 participants