-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Support yarn.lock maintenance #105
Conversation
Codecov Report
@@ Coverage Diff @@
## master #105 +/- ##
==========================================
- Coverage 69.92% 67.93% -1.99%
==========================================
Files 19 19
Lines 838 864 +26
Branches 140 143 +3
==========================================
+ Hits 586 587 +1
- Misses 252 277 +25
Continue to review full report at Codecov.
|
@hbetts this works when there's one I think there are two main options:
This is also actually a problem for regular Given that package file paths are incompatible with branch naming, we need another approach, such as counters. Example: However, the approach should be deterministic so that we can be sure we get the same counter for a package file every time it runs. Users might add or remove package.json / yarn.lock files so we can't be sure that counters would remain the same forever, although this potentially isn't an issue if renovate can "heal" PRs that were open prior to this. An alternative to counters would be to use a short hash of the package.json path, which would ensure that it is the same forever, even if other package.json files are added or removed. Unfortunately all the above options make the branch/pr naming less human-understandable. Perhaps best to support a single branch/PR for |
@jonbretman do you have any requirements or opinions on this feature? |
I was under the impression that I see the allure of option Coincidentally, Oddly they have a single global |
I was going to ask if you could elaborate on that statement, since I was under the impression that you could use full paths (Though I don't imagine full paths would be easy to read in a branch name). However, it seems a git branch name can't end with
I agree that a hash, while a novel approach, would not create a good user experience. A hash is not only not readable, but from the branch name alone I can't tell what package within the monorepo is being targeted with an update. Actually, coming back to the branch name, we don't need the branch name to end with |
I guess what I'm getting at is perhaps we should engage the
|
It does.. in fact my main repo has three |
Re: |
I guess that "putting all yarn.lock updates in the same branch" is a reasonable first step to merge though and would satisfy all single-repo users plus perhaps most monorepo. If they are done as separate commits then it also gives users the option to cherry-pick them too. |
Also, once this feature is done we could add a configuration to disable changes to |
|
0e3ace9
to
c8cfaec
Compare
lib/worker.js
Outdated
return; | ||
} | ||
logger.debug('Yarn lock needs updating'); | ||
const commitMessage = 'Renovate yarn.lock file'; |
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.
Could this be added to the default definitions so that it can be configured by the project or renovate
user? (We use Angular commit conventions for all our commits to help with other automation tooling)
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.
Yes, good idea. I'm a little concerned about the "scalability" of too many message definitions, however that's not an excuse for making them hardcoded!
@rarkins is this pull request in a state where I can begin testing it? |
# Conflicts: # lib/helpers/yarn.js
@destroyerofbuilds this is now ready for testing. I have made the four strings in use (branch name, commit message, pr title, pr body) all configurable although didn't personally test they were working. Otherwise I've tested it on a repo with a single
It's harmless - the PR can be closed without merge, or left around until a day or week later when it does have a diff. It would be a nice-to-have feature to detect when an existing yarn lock maintenance PR can be closed automatically though - perhaps a future issue. Also as noted in the documentation, this is only known to work when a repo has a single |
# Conflicts: # docs/configuration.md # lib/config/definitions.js # readme.md
description: 'Commit message template when maintaining yarn.lock', | ||
type: 'string', | ||
default: 'Renovate yarn.lock file', | ||
cli: false, |
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 just realized this option is blocked on the command line.
Kept wondering why I couldn't specify it.
Would/could this be opened up so that it's available to be specified on the command line?
This PR will close #96
It regenerates each
yarn.lock
file it finds and raises a PR if differences are found.Still to do: