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

Upgrades the version of black used in CI #131

Merged
merged 7 commits into from
May 29, 2024

Conversation

ilumsden
Copy link
Collaborator

@ilumsden ilumsden commented May 24, 2024

@draganagrbicllnl has been running into issues with black while trying to get formatting working in her PR #126. After looking at our CI, I realized we were using a 2.5 year old version of black with quite a few known bugs. The version of black we were using was particularly problematic since it was a beta version.

This PR upgrades black in CI to the newest version that supports our target Python version (3.9).

@ilumsden ilumsden self-assigned this May 24, 2024
@ilumsden ilumsden added area-ci Issues and PRs related to continuous integration processes used by Hatchet developers priority-normal Normal priority issues and PRs status-work-in-progress PR is currently being worked on type-internal-cleanup PR or issues related to the structure of the codebase, directories and refactors type-style labels May 24, 2024
Copy link
Collaborator

@pearce8 pearce8 left a comment

Choose a reason for hiding this comment

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

@ilumsden It looks like you updated black - but reverted to the old version of flake. Why? If this is as intended, please update the PR title and description to have the desired version(s), and put the reason(s) for the version choice in the description.

@ilumsden ilumsden changed the title Upgrades the version of flake8 and black used in CI Upgrades the version of black used in CI May 27, 2024
@ilumsden
Copy link
Collaborator Author

@pearce8 I've updated the title and description.

I was originally going to upgrade flake8 as well, but every newer version I tried had some type of internal error. This is likely to do a version mismatch between flake8 and one of the other tools it uses behind the scenes. flake8 is really just a thin wrapper around 3 or 4 other formatting tools by the same authors. The biggest problem with flake8 is that its dependencies on those other tools are hyper-specific. Sometimes when those dependencies are off even by a patch version, flake8 breaks when you try to run it.

Regarding the part of your comment asking for a "reason for the version choice", the last sentence (shown below) covers that:

This PR upgrades black in CI to the newest version that supports our target Python version (3.9).

There wasn't any other complex reason for the choice. It was just the newest version supporting the Python version that we use when running black and flake8 in CI. It is also coincidentally the newest version of black in general.

@ilumsden
Copy link
Collaborator Author

ilumsden commented May 27, 2024

FWIW, the issues I ran into with flake8 are also a big reason why a lot of developers are looking different tools for formatting and linting in Python (e.g., ruff).

@pearce8 pearce8 merged commit 9a4e09f into LLNL:develop May 29, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ci Issues and PRs related to continuous integration processes used by Hatchet developers priority-normal Normal priority issues and PRs status-work-in-progress PR is currently being worked on type-internal-cleanup PR or issues related to the structure of the codebase, directories and refactors type-style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants