EndangeredMassa / npub

publisher for node.js projects
BSD 3-Clause "New" or "Revised" License
3 stars 2 forks source link

initial publish flow #1

Closed EndangeredMassa closed 10 years ago

EndangeredMassa commented 10 years ago

npub version 1.2.3

(in order)


broken off into other issues

The config should allow overrides, such as which remote you push to and what verification step to run (instead of npm test).

The actual github interactions would be the hardest part. We could do something like https://github.com/kpdecker/git-todo#example to connect to github as the proper user, at least.

EndangeredMassa commented 10 years ago

@jkrems I want to make this do everything we want. Thoughts?

jkrems commented 10 years ago

Wishes:

Sorry, already a little late - hope this list kind of makes sense.

EndangeredMassa commented 10 years ago

For the verification step, how would that be handled? It needs to happen so early in the process that you might as well manually run it yourself first. If it is run by npub, like you said it could default to npm test, but specifying anything more complex would require a more complex invocation (in which case you could have just easily chained your commands with &&) or another change to the package.json with that command.

I am leaning towards just leaving out any verification step. Do you think we should really include that here?

EndangeredMassa commented 10 years ago

Updated the plan.

jkrems commented 10 years ago

The reason why I mentioned the verification step is that I'd hope to get to a point where we can ditch a requirement on Makefiles. Not get rid of Makefiles but at least have npub being usable without them. I feel like without the ability to run a verification step, npub wouldn't represent best publish practice. You shouldn't trust that you checked out a branch/commit that CI was a-ok with. So either we say "npub is not meant to be used without supporting other tools like make" or it should offer a verification step before publishing. This could be as easy as checking the package.json for a test-prepublish or test entries and then shelling out to npm run test-prepublish and/or npm run test (depending on which is available). Not sure about the name of the script, but you get the gist.

I know I'm more on the paranoid side of things, but it would make me very happy if npub wouldn't let anyone publish without passing tests. The ability to provide a custom (most likely even more thorough) test would be the cherry on top.

jkrems commented 10 years ago

In other words: it should be a step one. "Use &&" could also be said about the initial dirty check, I think it's the same category. Sanity checks to make sure the current state can be published.

EndangeredMassa commented 10 years ago

Sounds good to me. We can use a package.json value that defaults to npm test if not set.

EndangeredMassa commented 10 years ago

Implemented a basic publishing flow. Needs some work, but it does work at a basic level!

EndangeredMassa commented 10 years ago

We can get a list of PRs affected by identifying the commit range (by inspecting tags) and then hitting the commit list API endpoint and looking for the "Merge pull request #999" messages. Thanks @jkrems!

EndangeredMassa commented 10 years ago

I recently found a couple of similar modules, trapped in grunt packages. They may be useful for reference.

https://github.com/geddski/grunt-release

https://github.com/boennemann/grunt-semantic-release

EndangeredMassa commented 10 years ago
dbushong commented 10 years ago

FWIW, I've used grunt-release and not actually found it that useful. It's fine for patch releases, but it

  1. doesn't let you configure the tag, so no vX.Y.Z, only X.Y.Z
  2. doesn't play well when you've already set package.json to the version you want; it insists on incrementing, so I don't end up using it for anything but patch releases
  3. doesn't have much configurability for which remote you push the tag to (upstream vs. origin)
jkrems commented 10 years ago

Other problems with grunt-release:

:trollface:

EndangeredMassa commented 10 years ago

Sure. I just thought they might have a thing or two we could use.

The problem with both (before we event get to functionality) is that they are packages of logic trapped in task runner tasks. The logic should be it's own library that happens to be used in a grunt-* wrapper for grunt.

EndangeredMassa commented 10 years ago

npm test will now be called to make sure everything is in a good state

abloom commented 10 years ago

@EndangeredMassa what can I do to help finish this up and get it merged?

EndangeredMassa commented 10 years ago

I have an issue with the changelog that I'm trying to fix locally. Once that's done, I think it's good to merge. I should be able to get back to that soon.

EndangeredMassa commented 10 years ago

Ready for review. This now works as expected.

The callback chain should probably be reduced to an async.waterfall.

EndangeredMassa commented 10 years ago

Thoughts here?

EndangeredMassa commented 10 years ago

updated via @abloom

abloom commented 10 years ago

:shipit: