Add flag to fail publishing if version conflict in NPM

[jeanml]

Builds can silently fail if plugin tries to publish a package version that already exists in the target registry. To avoid this, this pull request adds an additional flag that forces the plugin to fail if there is a version conflict. Defaults to existing behavior if not specified.

Usage:

docker run --rm \
  -e NPM_USERNAME=drone \
  -e NPM_PASSWORD=password \
  -e NPM_EMAIL=drone@drone.io \
  -e PLUGIN_FAIL_ON_VERSION_CONFLICT=true \
  -v $(pwd):$(pwd) \
  -w $(pwd) \
  plugins/npm

[tboerger]

Maybe name the attribute conflict_fail?

[jeanml]

I was actually thinking that this really should be the default behavior and suggesting renaming to ignore_conflict. By default that would be false and the step would fail if there is a version conflict.

I am fully aware that this is a breaking change but if it breaks anyone's build I'd like to think that it's a good thing. Any thoughts?

[donny-dont]

@jeanml let me give some context on why it was done this way.

The NPM plugin was created over 3 years ago, which is around the Drone 0.3 release, so its one of the oldest plugins for Drone. At the time I don't believe that when you enabled Drone on a repo that it would actually build tags by default. So it assumed that someone using it wouldn't know that they needed to turn on that within the settings. I'm also not sure how robust the conditionals were at that time.

This was a decision made on usability.

I don't know the state of 1.0 with regards to what it builds. I don't see those options where you can turn off and on tags in cloud.drone.io for a repo I enabled. So maybe now it just turns everything on by default.

Now I agree with you that in an ideal world people should just have a when clause looking for a tag and only run the plugin on those events. However looking at Drone Plugins the NPM plugin is the 5th most downloaded plugin (the stuff with drone- prefixed are pre 0.5 plugins) so its really hard to introduce that sort of breaking change.

There are some breaking changes I'd like to do to the yaml for s3-cache but that one as well is used substantially. I'm not sure what exactly one would want to do here.

@tboerger @bradrydzewski is there anywhere to discuss breaking changes like this in a plugin?

[tboerger]

Discussions are best on discourse. I personally wouldn't add that breaking change.

[jeanml]

@donny-dont Not sure I am totally following you on your explanation relating to tags. What I really want to avoid here is an NPM publish Drone step silently failing to publish (in our case, to an internal Artifactory NPM registry) if there is a version conflict.

In any case, I totally get that given the popularity of the plugin, introducing a potentially (it only fails if you do actually have a version conflict) breaking change is perhaps not desirable.

The code change in the pull request is non-breaking and requires opting-in to fail on version conflict. Happy to go with that option as well! I don't mind renaming the plugin property to conflict_fail as suggested by @tboerger or fail_on_version_conflict or leaving it as is (fail_if_conflict).

[tboerger]

I'm for this current non-breaking behavior. Now it's just a matter of taste how this attribute got to be named :)

[jeanml]

I have updated the pull request to use fail_on_version_conflict. Ready to merge as far as I'm concerned if you are happy with the change.

[donny-dont]

@jeanml the correct way to do things would probably be.

pipeline:
  ...

  publish:
    image: plugins/npm
    ...
    when:
      event: tag

The way things are done now is without the when tag because as I mentioned tags were not on by default when you enabled a repo. You had to go into the settings and enable them explicitly.