Add bump version command that creates a git tag and a bower.json file (issue #203)

[Dretch]

Description of the change

Implements #203. Not yet feature complete.

Checklist:

• Added the change to the "Unreleased" section of the changelog
• Added some example of the new feature to the README
• Added a test for the contribution (if applicable)

[Dretch]

It looks like there are quite a few conflicts with #272 -- once that is merged I will resolve them.

[f-f]

@Dretch this looks quite good already, while I think #272 will need some more work, so I'd say we could try to merge this first and then rebase that one? (cc @jonathanlking)

What's missing from here to be feature complete?

[jonathanlking]

@Dretch @f-f please go ahead and merge this first - I’m very happy to rebase and I still need to do some work.

[Dretch]

@f-f

What's missing from here to be feature complete?

Right now I thinking of:

• Taking the license and repository field from spago.dhall and putting them in the generated bower.json.
• Readme and --help message tweaks
• Moving all messages into Messages.hs
• Maybe handling hash and branch versions from the package set (right now I think this will just create a broken bower.json)
• Maybe running bower install after generating the bower.json, as a sanity check.
• Maybe more tests.

[f-f]

@Dretch thanks for the summary. Some suggestions:

Taking the license and repository field from spago.dhall and putting them in the generated bower.json

For doing this let's add an ensurePublishConfig nearby here that tries to conform to a new PublishConfig type

Moving all messages into Messages.hs

As time goes I'm less sure the Messages module is useful, as you lose a bit of context if the relevant error message is one indirection away. So far the general attitude in my PRs has been to only put there multiline messages, and keep short ones in the relevant places. But I'd say to not worry much about all of this

Maybe handling hash and branch versions from the package set (right now I think this will just create a broken bower.json)

Right, great point. Fortunately we have some facilities for this already 🎉
You could use getMetadata (we already use this for the global cache) to get a map containing (among other things) a list of tags for every package in the set, so you could just check if all the refs are in there.

[Dretch]

Thanks for the feedback @f-f

For doing this let's add an ensurePublishConfig nearby here that tries to conform to a new PublishConfig type

I will look at doing this.

As time goes I'm less sure the Messages module is useful, as you lose a bit of context if the relevant error message is one indirection away. So far the general attitude in my PRs has been to only put there multiline messages, and keep short ones in the relevant places. But I'd say to not worry much about all of this

Okay, great.

Right, great point. Fortunately we have some facilities for this already. You could use getMetadata (we already use this for the global cache) to get a map containing (among other things) a list of tags for every package in the set, so you could just check if all the refs are in there.

Okay, I'll look at getMetadata. There are two options I can think of for handling non-tagged versions, though:

1. Fail when generating the bower.json if the package set refers to any such versions.
2. Generate a bower.json with Git URL versions, like:

dependencies: {
  "mkdirp": "https://github.com/joshuahhh/purescript-mkdirp.git#master"
}

Do you have any preference?

EDIT: Also, it looks like it will be necessary to call bower install, because otherwise pulp publish complains it can't read the resolved dependency versions from bower_components.

[Dretch]

🤷‍♂

[f-f]

@Dretch yeah, I also have no idea what's going on. The closest thing I found is this issue, that mentions that inproc could be a problem here (we use inproc for Git and procs for Bower)

[f-f]

@Dretch yeah it looks like 3c14203 fixed it 🎉

[f-f]

@Dretch looks great, thanks for doing this! 👏🎉

Ping @hdgarrood for a quick review, but I'd say we should merge already (so we can rebase #301, #272 and #283 on this) and address feedback in subsequent PRs

[hdgarrood]

This looks reasonable to me, but if I am reading correctly I think this is currently assuming that a package called foo in your package set is always the same as the package purescript-foo on Bower? That's only a safe assumption if you're using an official package set with no additions or overrides, right?

[hdgarrood]

(Not that this should prevent this from being merged, just something that's probably worth being aware of.)

[f-f]

@hdgarrood yeah that's right. I think this assumption is reasonable as we use it around already (e.g. when suggesting deps names), though your comment made me realise that with this implementation you'd be allowed to publish a package that only depends on packages in the official set because we search for every dependency's tag in the "packages metadata", which is a big map containing all tags and commits for all packages in the official package set

So maybe we should just generate the bower file and try to install and see if bower complains? (cc @Dretch)

[hdgarrood]

So maybe we should just generate the bower file and try to install and see if bower complains? (cc @Dretch)

Oh yes of course, this PR does this already -- it's necessary for pulp publish to succeed -- so that should be fine.

[f-f]

@hdgarrood yeah, what I meant there is that we should remove the check here otherwise one wouldn't be able to publish anything that doesn't depend from the packages in the package set.