Closed bmos closed 3 months ago
I think this should be split into several different PRs for the different components here. Each PR should explain the motivation for the relevant changes and what is affected.
I see these as separate PR scopes:
Good work though, these look mostly like nice updates. I think we'd want to discuss pros and cons of switching to uv, and I think it would be preferable to stick to the black line length default of 88 rather than switching it to the flake8 default of 100, but we should have those discussions in smaller scoped PRs.
Another note is to make sure that any PRs with breaking changes are set up to merge into the major-release branch rather than main. Mostly looking at the dropping support for 3.7.
Thanks for those notes, I will restructure this into the suggested categories.
I think it would be preferable to stick to the black line length default of 88 rather than switching it to the flake8 default of 100
100 is not the default for flake8, that would be 79. I went with 100 because the flake8 config file in your repo says 100 and the CONTRIBUTING.md file also includes it in the flake8 command. If 88 is the desired value I can change those values of 100 to 88 -- standardization was my goal, not a longer line length.
Thanks for those notes, I will restructure this into the suggested categories.
I think it would be preferable to stick to the black line length default of 88 rather than switching it to the flake8 default of 100
100 is not the default for flake8, that would be 79. I went with 100 because the flake8 config file in your repo says 100 and the CONTRIBUTING.md file also includes it in the flake8 command. If 88 is the desired value I can change those values of 100 to 88 -- standardization was my goal, not a longer line length.
Oh word gotcha that makes sense! Yeah I think maintaining whatever the documented value is makes the most sense so that sounds right to me.
Does the major release branch need to be brought up to main?
It doth, yes. It will be eventually, PRs against it will look a little messy until that happens.
On Wed, Mar 13, 2024, 8:06 PM Wil T @.***> wrote:
Does the major release branch need to be brought up to main?
— Reply to this email directly, view it on GitHub https://github.com/move-coop/parsons/pull/1009#issuecomment-1996307319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO74QHX3ZANUFFE4Y337AFDYYEH4BAVCNFSM6AAAAABES6PVC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJWGMYDOMZRHE . You are receiving this because you commented.Message ID: @.***>
I'm not really sure how these changes could be merged nicely with other open PRs. I'm happy to restructure it however is desired, so perhaps treat this as a sort of menu.
What's Included
__init__.py
(which is kind of pointless but I did it while debugging an error I was getting) and fixed a typo in it (one of the tuples was a list instead).Notes / Considerations