Skip to content
This repository has been archived by the owner on Sep 15, 2020. It is now read-only.

Initial implementation #1

Merged
merged 20 commits into from
May 1, 2018
Merged

Initial implementation #1

merged 20 commits into from
May 1, 2018

Conversation

donny-dont
Copy link
Collaborator

Here is an implementation of a drone plugin that uses greenkeeper-lockfile. Greenkeeper is a way to keep NPM dependencies up to date and this particular repository is used to keep lockfiles up to date as well. Its used pretty heavily in the JS community and has an enterprise offering so it'd probably be a good thing to support it all.

@bradrydzewski @tboerger it needs to be added to the hub.

Copy link
Collaborator Author

@donny-dont donny-dont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will test some more when its merged but so far its working locally.


RUN apk add --no-cache git nodejs nodejs-npm yarn
RUN npm install -g npm@latest \
npm install -g donny-dont/greenkeeper-lockfile#feat/drone-ci
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting on greenkeeperio/greenkeeper-lockfile#141 to merge before changing the endpoint.

Copy link
Contributor

@tboerger tboerger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nitpicking, good job

Gopkg.toml Outdated
@@ -0,0 +1,38 @@
# Gopkg.toml example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have dropped these comments from Gopkg.toml

@@ -0,0 +1,62 @@
# drone-greenkeeper

[![Build Status](http://beta.drone.io/api/badges/drone-plugins/drone-greenkeeper/status.svg)](http://beta.drone.io/drone-plugins/drone-greenkeeper)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add all badges, including discourse and stackoverflow.

main.go Outdated
"github.com/urfave/cli"
)

var build = "0" // build number set at compile-time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the build number and version.

main.go Outdated
app.Name = "npm plugin"
app.Usage = "npm plugin"
app.Action = run
app.Version = fmt.Sprintf("1.0.0+%s", build)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the build number and version from above, it will be set by drone tags.

main.go Outdated
cli.BoolFlag{
Name: "github_token",
Usage: "Github token",
EnvVar: "PLUGIN_GITHUB_TOKEN,GREENKEEPER_GITHUB_TOKEN",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add GITHUB_TOKEN since it's common for github auth?

plugin.go Outdated
log.Info("Anonymous NPM credentials are being used")
}

var cmds []*exec.Cmd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this slice? Just directly execute one of the 2 commands.

plugin.go Outdated
}

// See if authentication is required
if len(p.Config.Username) > 0 || len(p.Config.Token) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do so many people check the length of strings? Just string == "" :)

@donny-dont
Copy link
Collaborator Author

@tboerger I think its ready for a final review.

// uploadCommand runs greenkeeper-lockfile-upload.
func uploadCommand(gk Greenkeeper, build Build) *exec.Cmd {
cmd := exec.Command("greenkeeper-lockfile-upload")
cmd.Env = append(droneEnvironment(build), greenkeeperEnvironment(gk)...)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just a heads up with this. The greenkeeper-lockfile stuff expects certain environment variables to be present. One piece of that is stuff detected when it finds that its in the Drone CI environment. The other piece of that is config for the actual commands.

Idea was to just jam all this into an environment in the command rather than appending to the local environment variables.

@donny-dont donny-dont merged commit 4d7bc4e into master May 1, 2018
@tboerger tboerger deleted the feat/impl branch May 1, 2018 15:36
@tboerger
Copy link
Contributor

tboerger commented May 1, 2018

The secrets are already added?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants