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

Update build system to fail more quickly #424

Open
sobolevnrm opened this issue Dec 30, 2024 · 6 comments
Open

Update build system to fail more quickly #424

sobolevnrm opened this issue Dec 30, 2024 · 6 comments

Comments

@sobolevnrm
Copy link
Member

Update GitHub workflows to fail quickly on linting errors before running the more time-consuming functional tests.

@kiyoon
Copy link
Collaborator

kiyoon commented Dec 30, 2024

Or, you can separate the test to a different workflow to identify which part is failing more clearly

@sobolevnrm
Copy link
Member Author

@kiyoon can you please explain the differences between these three instances of the ruff linting checks?

  1. - name: Lint with ruff

@kiyoon
Copy link
Collaborator

kiyoon commented Dec 31, 2024

  1. It's a syntax error check. It already existed in this project in the CI with flake8, so I kept the same rules.
  2. You can fix by ruff format . (apply ruff formatter)
  3. You can fix by ruff check --select I --fix . (fix ruff import sorting lint)

So 2, 3 are not as important as 1.

@kiyoon
Copy link
Collaborator

kiyoon commented Dec 31, 2024

If you go in the action, there is a summary of the run where you can see the diff output of how you would fix them. Currecntly they all pass so there's no output here

image

@sobolevnrm
Copy link
Member Author

Yes, I understand how to view the output. I was just asking why there were 3 different linting tests. It seems like the first test duplicates the other two.

@kiyoon
Copy link
Collaborator

kiyoon commented Dec 31, 2024

The first test is completely different from the other two, but if you wish you can combine all of them in one. The problem is that there are important tests and there are just style checks. If too many things are combined, maybe it's hard to review others code and ask them to fix stuff, for that I preferred separated 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

No branches or pull requests

2 participants