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

Update devcontainer #1468

Merged
merged 12 commits into from
Jun 6, 2023
Merged

Update devcontainer #1468

merged 12 commits into from
Jun 6, 2023

Conversation

bdevans
Copy link
Contributor

@bdevans bdevans commented May 15, 2023

This includes a few fixes and bumps to the developer packages used.

Copy link
Member

@mstimberg mstimberg left a comment

Choose a reason for hiding this comment

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

Thanks for the version updates and fixes. The newer versions made the pre-commit hook fail, though, so I committed a few additional changes. The newer flake8-bugbears version flagged some (false positive) issues, I corrected/ignored them in the source code. The newer black version made a large number of changes to multi-line strings (since we use the --preview flag). I'd rather not commit these changes, since it might change again in the near future. For now, I'd suggest that we stay with the previous version of black until the "preview style" becomes the new stable style (planned for beginning of 2024). I also installed the flake8 extension in the dev container so that Visual Code stops nagging me about it ;-) If all that looks ok for you, happy to merge.

@mstimberg
Copy link
Member

👋 @bdevans did you have a chance to look at my changes?

@bdevans
Copy link
Contributor Author

bdevans commented Jun 6, 2023

Hi @mstimberg, the charges look fine except reverting to the previous black version. It seems like a good idea to keep up with best practices as implemented in the autoformatter, although it may be too big a change for this PR. I reread your previous comment and see you want to hold back until 2024 but this seems like a long wait - would it be a problem to upgrade incrementally?

@mstimberg
Copy link
Member

Hi @bdevans . No, please let's not change the formatting in our whole codebase with each update of black's preview style, let's wait until it is stable. See our previous discussion, e.g. #1435 (comment)

@mstimberg mstimberg merged commit ce4884f into master Jun 6, 2023
@mstimberg mstimberg deleted the update_devcontainer branch June 6, 2023 14:03
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.

2 participants