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

Python: py_samples could run blacken #899

Closed
tswast opened this issue Jan 8, 2021 · 3 comments · Fixed by googleapis/python-bigquery-reservation#71
Closed

Python: py_samples could run blacken #899

tswast opened this issue Jan 8, 2021 · 3 comments · Fixed by googleapis/python-bigquery-reservation#71
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tswast
Copy link
Contributor

tswast commented Jan 8, 2021

The noxfile.py in the samples template now includes a blacken session. It's great to run this, but I notice that the changes it makes get reverted the next time that synthtool runs. I think it makes sense for blacken to blacken the noxfile, but if used then it shouldn't get reverted.

Since I'm not sure if all of our samples are using black, I propose we add an optional parameter to py_samples that runs blacken when done copying everything.

Reference:

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 8, 2021
@tswast tswast added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed samples Issues that are directly related to samples. labels Jan 8, 2021
@tswast
Copy link
Contributor Author

tswast commented Jan 8, 2021

Confirmed that this needs to be an opt-in parameter.

@tswast tswast self-assigned this Jan 14, 2021
@tswast
Copy link
Contributor Author

tswast commented Jan 14, 2021

On second thought, I see that blacken is actually called via the shell command.

https://github.com/googleapis/python-bigquery-reservation/blob/2acdeb782521c01a4e1fa01e42fdd1ce79dbf13d/synth.py#L66

Perhaps what is needed is to do the same for each samples directory?

@tswast
Copy link
Contributor Author

tswast commented Jan 14, 2021

It was actually quite simple to blacken all samples. I don't think this requires any changes to synthtool.

import pathlib

REPO_ROOT = pathlib.Path(__file__).parent.absolute()

# ...

# ----------------------------------------------------------------------------
# Final cleanup
# ----------------------------------------------------------------------------

s.shell.run(["nox", "-s", "blacken"], hide_output=False)
for noxfile in REPO_ROOT.glob("samples/**/noxfile.py"):
    s.shell.run(["nox", "-s", "blacken"], cwd=noxfile.parent, hide_output=False)

@tswast tswast closed this as completed Jan 14, 2021
gcf-merge-on-green bot pushed a commit to googleapis/python-bigquery-reservation that referenced this issue Feb 3, 2021
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery-reservation/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Fixes googleapis/synthtool#899 🦕
Closes #66
parthea pushed a commit to googleapis/google-cloud-python that referenced this issue Sep 22, 2023
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery-reservation/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Fixes googleapis/synthtool#899 🦕
Closes googleapis/python-bigquery-reservation#66
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant