Skip to content

Update iSort to use Black style #267

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

Merged
merged 8 commits into from
Jul 11, 2023
Merged

Update iSort to use Black style #267

merged 8 commits into from
Jul 11, 2023

Conversation

dyastremsky
Copy link
Contributor

This pull request incorporates feedback from the pre-commit trial period now that we are rolling this out across all of our repos. Namely, it adds Black style to the iSort hook. It also normalizes the pre-commit-config spacing.

@dyastremsky dyastremsky marked this pull request as ready for review July 5, 2023 15:04
@nv-kmcgill53
Copy link
Contributor

This goes as a note for all other open precommit PRs, but we should be consistent across all repositories regarding decisions being made such as removing redundant specs. I don't know if you want me to spam this for all other PRs or how you want to track this and any other suggestions which are not repository specific. Let me know.

@dyastremsky
Copy link
Contributor Author

This goes as a note for all other open precommit PRs, but we should be consistent across all repositories regarding decisions being made such as removing redundant specs. I don't know if you want me to spam this for all other PRs or how you want to track this and any other suggestions which are not repository specific. Let me know.

Thanks, Kyle. That's the plan. Commenting on one PR is enough. I am waiting to see if the GitHub team gets back to me about why their code says that pull requests need to be a subset of push before copying out the changes to all the other PRs (which would not respect that commented but undocumented rule). If not, I'll copy it out anyway soon as the change looks to be working as expected in test PRs as well as on that PR itself (you can see the GitHub actions run correctly after the commit).

@dyastremsky
Copy link
Contributor Author

dyastremsky commented Jul 6, 2023

This goes as a note for all other open precommit PRs, but we should be consistent across all repositories regarding decisions being made such as removing redundant specs. I don't know if you want me to spam this for all other PRs or how you want to track this and any other suggestions which are not repository specific. Let me know.

Thanks, Kyle. That's the plan. Commenting on one PR is enough. I am waiting to see if the GitHub team gets back to me about why their code says that pull requests need to be a subset of push before copying out the changes to all the other PRs (which would not respect that commented but undocumented rule). If not, I'll copy it out anyway soon as the change looks to be working as expected in test PRs as well as on that PR itself (you can see the GitHub actions run correctly after the commit).

As an update, I have not heard back from GitHub but copied the changes across the repos for consistency. The workflows are working as expected.

Update: GitHub responded that the comment was an issue with their code editor, so that is not a rule. Our approach works.

@dyastremsky dyastremsky requested a review from Tabrizian July 10, 2023 22:03
@dyastremsky dyastremsky merged commit 8c93267 into main Jul 11, 2023
@dyastremsky dyastremsky deleted the dyas-precommit branch July 11, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants