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

Greenkeeper does not update package-lock.json #1415

Closed
platan opened this issue Jan 3, 2018 · 21 comments
Closed

Greenkeeper does not update package-lock.json #1415

platan opened this issue Jan 3, 2018 · 21 comments
Labels
bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI

Comments

@platan
Copy link
Member

platan commented Jan 3, 2018

A recently example #1414

@paulmelnikow paulmelnikow added bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI labels Jan 3, 2018
@paulmelnikow
Copy link
Member

greenkeeper-lockfile is supposed to handle this, though I guess it's not working correctly.

@tooomm
Copy link
Contributor

tooomm commented Jan 3, 2018

Shouldn't you bring that up at https://github.com/greenkeeperio/greenkeeper instead @platan?

@platan
Copy link
Member Author

platan commented Jan 3, 2018

Either greenkeeper (greenkeeper-lockfile) does not work or we are using it incorrectly (bad configuration). Greenkeeper was introduced when we were using Travis CI. Now we have Circle CI.

@paulmelnikow
Copy link
Member

Just noticed this is on file upstream: greenkeeperio/greenkeeper-lockfile#58

@platan
Copy link
Member Author

platan commented Jun 3, 2018

I think we can close this issue. Greenkeeper updated package-log.json in the latest PRs, e.g. #1716

@platan platan closed this as completed Jun 3, 2018
@chris48s
Copy link
Member

chris48s commented Jun 3, 2018

no - this is still an issue. I updated the lockfile locally on those PRs and pushed the extra commits to the greenkeeper branches

@chris48s chris48s reopened this Jun 3, 2018
@platan
Copy link
Member Author

platan commented Jun 4, 2018

@chris48s oh, my bad, I overlooked your commit. Thank you!

@paulmelnikow
Copy link
Member

This is super frustrating! Given @jasonLaster's solution in greenkeeperio/greenkeeper-lockfile@master...jasonLaster:master (see greenkeeperio/greenkeeper-lockfile#58) I'm not sure why the fix from #1562 did not solve the problem.

@chris48s How much work do you want to put into fixing this? Shall we go ahead and fork greenkeeper-lockfile in the badges org, build and test a workable solution, and try to get it merged upstream?

@chris48s
Copy link
Member

Its one of those paper cut bugs, isn't it.
If you're enthusiastic about shaving this particular yak, go for it. Manually updating the package-lock as part of the review isn't the end of the world.. At least it means we know someone had to install the package locally before clicking the 'approve' button ;)

@paulmelnikow
Copy link
Member

Yea, it's irritating without having an easy fix.

Honestly I'm not so enthused. Though maybe there's a workaround? This is a CircleCI issue. Weren't we talking about adding Travis for code coverage reasons? Maybe we should add Travis for Greenkeeper, as silly as that might sound…

@chris48s
Copy link
Member

Another option might be to switch out greenkeeper for something else that does roughly the same job like dependabot and see if that is more compatible with Circle CI?

@paulmelnikow
Copy link
Member

Yea, a good idea. Either of those sounds fine to me.

@paulmelnikow
Copy link
Member

Been having some great luck with Dependabot lately.

@chris48s
Copy link
Member

Been having some great luck with Dependabot lately.

Agreed. I've tried out dependabot on a couple of other projects and I'm fairly convinced that dropping greenkeeper for dependabot is a good call here. It deals with updating your package-lock outside of CI so should abstract this whole issue. It also auto-resolves merge conflicts on the lock file (or attempts to) and some other nice stuff.

Switching to dependabot will probably result in more noise/review overhead as it will submit a PR for every patch version (although we can configure particular packages to auto-merge if the tests pass - this might be a useful option for some stuff like testing-related dev tooling), however I think this approach is more correct for projects like this that use a lockfile. Greenkeeper is more targetted to libraries.

In terms of getting it done, I'm happy to submit a PR to remove the greenkeeper-lockfile stuff from the CI build, but if I log into greenkeeper or dependabot, I don't think I can add or remove either of them because I'm a repo collaborator not an organisation member so someone else probably needs to do that bit.

@paulmelnikow
Copy link
Member

I don't have enough access on the org to install Dependabot, which I think requires owner permission on the org. I can't make you a member, either, for the same reason. Though it looks like, as an admin on the repo, I can remove Greenkeeper.

@paulmelnikow
Copy link
Member

I've emailed @espadrine asking him for access.

@espadrine
Copy link
Member

Huh, that's not normal. I thought you had the same access I have.

In https://github.com/orgs/badges/teams/shields/members (a team with the highest rights, Admin), I see both of us labeled "maintainer".

Do you have an error message that could help me give you the same access?

@espadrine
Copy link
Member

Can you try again? I made you owner of https://github.com/orgs/badges/people. I don't know if it should change anything; I thought GitHub switched to team-based access management to make that irrelevant. GitHub rights confuse me!

@paulmelnikow
Copy link
Member

It worked! Thank you!

@paulmelnikow
Copy link
Member

Okay, so Dependabot is turned on and Greenkeeper is turned off. I've also updated your access to the organization @platan @PyvesB @RedSparr0w @chris48s.

We'll probably have some cleanup to do, though let's close this issue out since it is now fixed.

@chris48s
Copy link
Member

chris48s commented Aug 1, 2018

Yep - that seems to have done the job :) Now that we've enabled dependabot, it will submit PRs bumping out dependencies to the latest release (capped to a max of 5 per day), so there will be some overhead of reviewing dependency bump PRs until everything is pinned to the latest version. It is updating the lockfile though.

@chris48s chris48s closed this as completed Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

No branches or pull requests

5 participants