-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
Configure Renovate #654
Configure Renovate #654
Conversation
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.
I think we can try the default settings for now and reduce the noise later when we better understand how renovate affects us?
6b695b4
to
e7d973f
Compare
Codecov Report
@@ Coverage Diff @@
## master #654 +/- ##
==========================================
- Coverage 99.31% 99.31% -0.01%
==========================================
Files 94 94
Lines 11014 11002 -12
Branches 790 789 -1
==========================================
- Hits 10939 10927 -12
Misses 56 56
Partials 19 19
Continue to review full report at Codecov.
|
Well, I enabled Before we can merge this I think we need to at least:
|
I note this doesn't support setup.py dependencies. |
Right, this is a limitation of all services like this, because there's no way to get those without actually executing setup.py. Hopefully someday python will have a somewhat less tangled way of handling this kind of package metadata, but in the mean time the workaround is that we list our general runtime dependencies in |
Using pip-compile from pip-tools is the best way to pin all indirect dependencies, but I think we need to make sure that renovate runs pip-compile itself? For example, say dependency A starts depending on dependency A1: you want to pin both A and A1 when we start depending on a new A version, and that's only possible with a new pip-compile run. If that's not an option, I guess only pinning direct dependencies is a better compromise? |
From the todo list:
I've added 8289d5b to this effect. renovate/* branches are now ignored in all repos. I don't think this can be configured per repository, but there's no reason to have renovate/* branches in forks, so that's OK? |
Yeah, codecov can get very confused when one PR is merged while another's CI is running. (Partly this is the fault of the CI services: they actually pick a base revision for each job at the time the job starts, so if master moves while the CI is running, then you can end up with e.g. the py35 and the py36 jobs actually testing different trees...) We know nothing here should be touching coverage and the codecov results make no sense, so oh well, ignore it. There's a reason codecov isn't marked as mandatory for merge :-)
Interesting point. I don't know if renovate (or any similar tools) pull in new pins when necessary. But, it's also not the end of the world if we have some deps missing from our requirements file... basically it just means that we continue to automatically test the latest version, and are at risk of being broken by their releases. Either this never happens, and it's fine, or else it eventually happens, and then we notice and add them to the requirements.txt-file-of-shame :-) |
Re: |
@Zac-HD that does sound like some fancy tech! should we delegate all this to you? :-) |
Yeah, what Hypothesis is doing is impressive. The 'push back to the PR magic' PR is HypothesisWorks/hypothesis#1562. So I moved test-requirements.txt and ci/rtd-requirements.txt to the pip-compile format, but maybe we should keep a .in version in order to 1/ keep track of the direct dependencies and 2/ keep the comments we had until now? Of course, that's what Hypothesis does: https://github.com/HypothesisWorks/hypothesis/tree/master/requirements :) And yeah, I would be happy to remove my commits here and let @Zac-HD work on this! 😄 |
|
@njsmith any preferences regarding the location of .in and .txt requirement files? Oh, and the PyPy builds have all failed with:
And indeed, astroid (a pylint dependency) only depends on typed_ast with CPython: https://github.com/PyCQA/astroid/blob/6acd49c33e0143da5ab8aaf8caca49d1d564467c/astroid/__pkginfo__.py#L33. I guess pip-compile should have been able to pick that up? |
No strong preferences on the .in files... they can go next to the .txt files I guess, unless someone has a better idea? That is unfortunate that pip-compile didn't preserve the environment marker... reading issues like jazzband/pip-tools#635 , it sounds like it is intended to preserve it? Maybe if we manually add it to requirements.in, and then re-run it? |
@Zac-HD okay, at least we tried. 😛 @njsmith Yeah, this is noted in the pip-tools 2.0.0 release notes:
So, yeah, thanks @JoergRittinger. :) I added typed_ast with its environment markers in the .in file. This has fixed the PyPy builds. Please take a look. |
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.
I think we need to run pip-compile
in CI, but otherwise looking good to me.
test-requirements.in
Outdated
@@ -0,0 +1,11 @@ | |||
pytest == 3.7.4 # for catchlog fixture |
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.
I think this pin is an artifact from recent pytest trouble; and should just be >= 3.3
.
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.
Wow, nice catch! Fixed. I made sure there were no others invalid changes like that in the .in files. Thanks.
|
||
branches: | ||
except: | ||
- /renovate/*/ |
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.
Problem here, actually: if we skip CI for these branches, we'll update to incompatible versions of our dependencies too!
We should just delete the exclusion. Ideally we would also rewrite our CI scripts to use the pinned versions everywhere and check they're consistent, but you might want to leave that to a future PR (or even push them onto me 😉)
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.
The problem is that renovate puts these branches directly into the main repo, so the CI services want to run CI twice – once b/c it's a branch in the main repo, and a second time b/c it's a PR. I think this should eliminate the first CI, but keep the second?
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.
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.
...Derp, yeah, that makes sense. (in my defence- no, wait, pyup does the same 🤦♂️)
(I'd be happy to leave the pip-compile check in CI for another PR.) |
OK, let's see what happens :-) |
Welcome to Renovate! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin.
🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged.
Detected Package Files
ci/rtd-requirements.txt
(pip_requirements)test-requirements.txt
(pip_requirements)Configuration
🔡 Renovate has detected a custom config for this PR. Feel free to post it to the Config Help repository if you have any doubts and would like it reviewed.
What to Expect
With your current configuration, Renovate will create 1 Pull Request:
Update dependency sphinx to v1.8.0
renovate/sphinx-1.x
==1.8.0
❓ If you have any questions, try reading the Docs, particularly the Getting Started section.
Also, you can post questions about your config in Renovate's Config Help repository.
This PR has been generated by Renovate Bot.