feat: Integration map and CircleCI

greenkeeperio / greenkeeper-lockfile

:lock: Your lockfile, up to date, all the time

https://greenkeeper.io

183 stars 73 forks source link

feat: Integration map and CircleCI #7

Closed SpainTrain closed 7 years ago

SpainTrain commented 7 years ago

Added integration mapping for adding new CI integrations
Added CircleCI integration

closes #5

TODO

[ ] Update README (add directions for CircleCI)

SpainTrain commented 7 years ago

FYI, I am still testing this, but feel free to provide feedback on the approach

SpainTrain commented 7 years ago

Reviewed 5 of 5 files at r1. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

boennemann commented 7 years ago

Hey @SpainTrain,

I had a brief look at this and I'm super excited, as this is exactly what I had in mind. Thanks for picking this up and executing it so exceptionally well.

I'll get to work on this again on Friday – I'll leave a proper review then. Can't wait to get this merged soon.

Thanks a lot, Stephan

SpainTrain commented 7 years ago

Reviewed 2 of 2 files at r2, 3 of 3 files at r3. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

SpainTrain commented 7 years ago

alright, was able to do some testing and looking solid. I have not tested on Travis though.

SpainTrain commented 7 years ago

I realized I also need to update the README with directions for CircleCI

boennemann commented 7 years ago

Hey @SpainTrain,

I added some comments. Regarding the README – don't worry too much. If you don't want to spend time on it I can take care of if before merging – just let me know :)

Also: While using the current implementation I found a bug with the current approach of detecting whether the update was already applied. I think I found a super elegant solution for this on Travis, which I'd love to ship now, in order to unbreak current usage. If I land this I'll link up the changes here, so we can look into rebasing this branch onto the changes.

Best, Stephan

boennemann commented 7 years ago

Alrighty, I just pushed the update. I found a lot more to refactor and given how much our branches diverged I started to feel bad for you here. I just pushed a branch where I've already resolved the merge conflicts so it's not too hard for you to pick this up again: https://github.com/greenkeeperio/greenkeeper-shrinkwrap/commits/integration-mapping-rebased

The most relevant change is this one: https://github.com/greenkeeperio/greenkeeper-shrinkwrap/commit/be944c0b56e9ffdd33941315ba6c01b28ba381b0

You would have to find an equivalent way of detecting this on CircleCI, and then add it to value mapping.

Best, Stephan

SpainTrain commented 7 years ago

Thanks for the review. All good points. I am currently slammed with work coding, but will hopefully get to rebasing and addressing comments in a week or so!

SpainTrain commented 7 years ago

Haven't forgot about this, just slammed on other stuff. Hopefully will be able to update quite soon!

boennemann commented 7 years ago

Hey @SpainTrain,

thanks for your effort with this PR. I went ahead and added the mapping. I'd be happy to have your Circle CI contribution: https://github.com/greenkeeperio/greenkeeper-lockfile#contributing-a-ci-service

Best, Stephan