Skip to content

Conversation

AlexWaygood
Copy link
Member

@Akuli: this is an attempt to assert in CI some of the invariants to do with our pyrightconfig files that are relied on by create_baseline_stubs.py and runtests.py (following up on #10629). Not sure if it's worth it or not; what do you think?

@AlexWaygood AlexWaygood requested a review from Akuli September 5, 2023 11:13
@Akuli
Copy link
Collaborator

Akuli commented Sep 5, 2023

IMO this is not worth the extra code. We lose some functionality (can no longer do inline comments), we add a lot of code, and we don't really fix any problem as far as I can tell.

Because create_baseline_stubs.py now sorts the list of third-party stubs, getting the list out-of-order would mean that the order gets restored next time someone adds new stubs. I think this is fine, because it is unlikely to get out of order in the first place: you typically use the script whenever adding to the list, and removing from the list doesn't mess up ordering.

Also, with this PR it's tempting to insert comments between entries of the exclude list. This would break create_baseline_stubs.py, but it works even if some lines have inline comments at the end.

@AlexWaygood
Copy link
Member Author

Sure, let's close this then. I thought it would be less code than it ended up being :)

@AlexWaygood AlexWaygood closed this Sep 5, 2023
@AlexWaygood AlexWaygood deleted the consistent-pyrightconfig branch September 5, 2023 13:43
@Akuli
Copy link
Collaborator

Akuli commented Sep 5, 2023

I thought it would be less code than it ended up being :)

This happened to me many times in #10629 before I came up with what I committed :)

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Sep 5, 2023

We lose some functionality (can no longer do inline comments), we add a lot of code, and we don't really fix any problem as far as I can tell.

oh, one problem it does fix: if we added inline comments to pyrightconfig.stricter.json, runtests.py would break, but we wouldn't realise unless we ran it locally since we don't run that script in CI. Possibly we could get rid of most of this code and just assert that there are no inline comments in pyrightconfig.stricter.json? Maybe even that isn't a massive issue, though.

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