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

CI: only run backend lints once, not in all test environments #3960

Merged
merged 2 commits into from Apr 26, 2023

Conversation

akx
Copy link
Contributor

@akx akx commented Apr 24, 2023

Description

No need to spend cycles running lints multiple times; just run it once in a separate step.

Checklist:

  • I have performed a self-review of my own code
  • I have added a short summary of my change to the CHANGELOG.md
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@gradio-pr-bot
Copy link
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-3960-all-demos

@akx akx force-pushed the ci-lint-only-once branch 3 times, most recently from 2e0ff36 to 6d33c04 Compare April 24, 2023 21:49
@akx akx changed the title CI: only run backend lint once, not in all test environments CI: only run backend lints once, not in all test environments Apr 24, 2023
@akx akx marked this pull request as ready for review April 24, 2023 21:53
@abidlabs
Copy link
Member

Hi @akx thanks yet again! This change looks fine to me, but let's run linting before we run the tests. Linting is much faster than running the tests, so better for it to fail quickly if the code is not linted rather than having to wait for all of the tests to run

@akx
Copy link
Contributor Author

akx commented Apr 25, 2023

@abidlabs @freddyaboulton Done deal. client-test needs client-lint, test needs lint now.

Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks @akx !

@freddyaboulton
Copy link
Collaborator

Can't update the branch and merge @akx - please merge in main and let me know so we can merge this in.

@akx akx force-pushed the ci-lint-only-once branch 2 times, most recently from a5f7794 to 1ec7e35 Compare April 26, 2023 07:58
@akx
Copy link
Contributor Author

akx commented Apr 26, 2023

@freddyaboulton Rebased, tests were flaky so I re-pushed, we'll see...

@freddyaboulton
Copy link
Collaborator

Hi @akx - can you reopen the PR and allow edits from maintainers? That way I can merge in main and get this merged! Otherwise might be tricky to align on timing

@akx
Copy link
Contributor Author

akx commented Apr 26, 2023

@freddyaboulton Sorry, enabled "allow edits" now and rebased.

@freddyaboulton
Copy link
Collaborator

Thanks again @akx !

@freddyaboulton freddyaboulton merged commit 37d52a7 into gradio-app:main Apr 26, 2023
17 checks passed
@abidlabs
Copy link
Member

Thanks @akx for addressing the feedback!

@akx akx mentioned this pull request Apr 27, 2023
@akx akx deleted the ci-lint-only-once branch June 27, 2023 10:42
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.

None yet

4 participants