-
Notifications
You must be signed in to change notification settings - Fork 85
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
Improve file filtering in CI #734
Conversation
scripts/nb-tester/test-notebook.py
Outdated
@@ -35,6 +35,36 @@ | |||
] | |||
|
|||
|
|||
def filter_paths(paths: list[Path], submit_jobs: bool) -> list[Path]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicer is to return Iterator[Path]
. Every time you want to return a path, you will yield path
. No need to have good_paths
as a collection - remove that. No return statement.
Lines 21 to 29 in ff3500a
def translation_volume_mounts() -> Iterator[str]: | |
"""Return the CLI args to mount each language in the translations/ folder. | |
Unlike the root `Dockerfile`, we cannot use `-v $PWD/translations:/home/node/app/docs` because | |
Docker complains that the volume is already mounted when we mount the `/docs` folder. So, instead | |
we need to be more specific to mount each language's folder. | |
""" | |
for folder in PWD.glob("translations/*"): | |
yield from ["-v", f"{folder}:/home/node/app/docs/{folder.name}"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much nicer, thanks!
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
The `transpiler-stages` notebook is broken, but will be tested in CI any time someone edits it (even if just changing copy etc.). This breaks PRs that are perfectly fine. This PR ignores excluded files completely. I also took the opportunity to: - improve the code readability somewhat (i.e. less `path for path in paths if path`), - print a message when notebooks are skipped, otherwise someone running `tox` locally might believe their notebook was fine even though it never ran. --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
The
transpiler-stages
notebook is broken, but will be tested in CI any time someone edits it (even if just changing copy etc.). This breaks PRs that are perfectly fine.This PR ignores excluded files completely. I also took the opportunity to:
path for path in paths if path
),tox
locally might believe their notebook was fine even though it never ran.