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

Reorder the commit hooks by stack depth. #9524

Merged

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Apr 12, 2020

Problem

There were two inversions in the commit hooks:

  1. lint and style checks were running before semantic checks for python. If you experienced a mypy type error, you were likely to make changes that would break style. And broken style would cause a context switch before you were able to see whether you had fixed the semantic issue.
  2. Likewise, python checks were occurring before rust checks: if the rust code was broken, you would need to re-compile it to run the python checks before being able to return to seeing whether you had fixed the semantic check.

These inversions can cause you to context switch between various tools to get back to the error you were trying to fix.

Solution

Re-order the hooks from bottom to top in terms of: 1) language (rust then python), 2) semantic level (semantic then stylistic).

[ci skip-rust-tests]
[ci skip-jvm-tests]

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thank you! MyPy seems to be the thing that has issues the most frequently, so it will be wonderful to run that before running the headers check, for example.

@stuhood stuhood merged commit 78a2de8 into pantsbuild:master Apr 13, 2020
@stuhood stuhood deleted the stuhood/reorder-hooks-based-on-stack-depth branch April 13, 2020 06:26
stuhood pushed a commit to stuhood/pants that referenced this pull request Apr 29, 2020
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
# Delete this line to force CI to run Clippy and the Rust tests.
[ci skip-rust-tests]
stuhood pushed a commit to stuhood/pants that referenced this pull request Apr 30, 2020
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
# Delete this line to force CI to run Clippy and the Rust tests.
[ci skip-rust-tests]
stuhood pushed a commit to stuhood/pants that referenced this pull request Apr 30, 2020
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
# Delete this line to force CI to run Clippy and the Rust tests.
[ci skip-rust-tests]
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.

3 participants