-
Notifications
You must be signed in to change notification settings - Fork 541
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
consolidate pytest config in pyproject.toml #6201
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test |
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.
Thanks for taking the time to figure this out and then fix it straight away.
Looks good to me.
Annoying that we can't add a pytest.ini
with a warning for people from the future to not use it :-/
@betatim we could add |
Good idea! |
Thanks for considering it! @betatim I just pushed that change, will you take a look and let me know what you think? I intentionally am leaving |
I also don't know exactly what it does/when it is used. But the robots (aka CI) seem happy with it in place so I'd also leave it alone for now. |
**/build.sh @rapidsai/cuml-cmake-codeowners | ||
CMakeLists.txt @rapidsai/cuml-cmake-codeowners | ||
*.cmake @rapidsai/cuml-cmake-codeowners | ||
**/cmake/ @rapidsai/cuml-cmake-codeowners |
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.
updated these rules based on an offline conversation with @vyasr and @dantegd
pyproject.toml
-only changes just need packaging-codeowners
(handled by the pyproject.toml
rule a few lines down).
This change also adds `cuml-cmake-codeowners:
- when any
.cmake
file is touched - when any
CMakeLists.txt
file is touched (regardless of depth in the filesystem)
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.
Looks good to me.
Not sure how to persuade the telemetry job to succeed. Just restarting it doesn't help. From the error message it sounds like it doesn't like being run a lot of time after other jobs it depends on? At least that is my explanation for why the artefact it is trying to download doesn't exist. I'd merge this as it is without trying to figure that one out. Assuming the optional jobs are truly optional (they fail because of #6209 so not something related to this PR) |
/merge |
fixes #6194
Wheel tests in this project are emitting tons of warnings like this:
I think that's because the introduction of a
pytest.ini
file in #6078 resulted in all of thepytest
options frompyproject.toml
being ignored.From https://docs.pytest.org/en/stable/reference/customize.html#pytest-ini
I think "take precedence" there means that if
pytest
finds apytest.ini
, it stops searching for other configuration files.