feat: Optimize linting by checking changed files - #5939
Conversation
another-rex
changed the title
another-rex
changed the title
michaelkedar
left a comment
michaelkedar
left a comment
There was a problem hiding this comment.
seems marginally over-engineered.
(also increasing the amount of Python in the repo 😔)
There was a problem hiding this comment.
I don't think this is related 🤔
There was a problem hiding this comment.
Huh... shouldn't this not be a problem since it's already merged in? Maybe merging master into this might fix this.
| # Dependency propagation for Go | ||
| if "bindings/go" in affected_mods and "go" in all_go_mods: | ||
| affected_mods.add("go") | ||
| if "go" in affected_mods: | ||
| for dep in ("go/cmd/tools/reimport-tui", "tools/repo-allowlist-sync"): | ||
| if dep in all_go_mods: | ||
| affected_mods.add(dep) |
There was a problem hiding this comment.
This hard-coded propagation is annoying to maintain, and I don't really understand its purpose - is it saying that if a dependency changes, then also lint the thing depending on it?
I don't really see why that would be necessary?
There was a problem hiding this comment.
Yeah idk, removed now.
| - If 'bindings/go' changes -> both 'bindings/go' and 'go' are linted. | ||
| - If 'go' changes -> 'go', 'go/cmd/tools/reimport-tui', and | ||
| 'tools/repo-allowlist-sync' are linted. | ||
| * Skipped (0.0s) when no in-scope Go modules changed. |
|
Also, generally you're using double quotes for strings, when the python style guide says single quotes should be used |
|
Simplified it a bit, now just finds all the targets (e.g. go.mod files), figures out the diffs and which belongs to which target, and then run the linter against the appropriate target. |
2 participants
Trying to do quick fixes really make you find out where the pain points in the CI.
Turned the lint and format script into a python script (with no dependencies). We now tune a few settings to make it run a lot faster.