purescript-contrib / governance

Guidelines and resources for the PureScript Contributors organization
15 stars 3 forks source link

Auto-fill CHANGELOG.md file with past release info #25

Closed JordanMartinez closed 3 years ago

JordanMartinez commented 3 years ago

Fixes #22

A few thoughts

JordanMartinez commented 3 years ago

Ok. I think this is good now.

thomashoneyman commented 3 years ago

A new CLI argument is now required on the contrib-updater tool: repo since I'm not sure whether we can always guarantee that the repo's name on GH matches the Spago name in the spago.dhall file.

We can't guarantee it, no, though it is the case for the contrib libraries. Can this be optional, defaulting to purescript-<spago.dhall name>?

The appendReleaseInfo function assumes that the CHANGELOG.md file was created in the runBaseTemplates call. In other words, the file creation and file appending don't occur within the same block together to prevent later separation. I don't like this, but I'm not sure whether it's worth it to refactor that

One thing we can do is just not generate a CHANGELOG in the first place and only use this autofill technique. After all, if we're going to basically overwrite it anyways, there's no need to generate the first one.

JordanMartinez commented 3 years ago

Can this be optional, defaulting to purescript-

Yeah, but I'm not sure whether that will cause more harm than good. I decided not to because it would create less problems for a few more characters typed. I'll switch to that approach.

thomashoneyman commented 3 years ago

I think that if we add the flag then we should no longer hardcode purescript-<package-name> in the template files and should use repo instead. For example, in the README files the badges, Pursuit links, and so on all use that format. They could all be changed to a {{repo}} variable instead.

JordanMartinez commented 3 years ago

Other changes I made:

thomashoneyman commented 3 years ago

This is quite nice! Just a couple minor comments but this will make a good addition.

thomashoneyman commented 3 years ago

@JordanMartinez do you recognize this error? I am seeing it on current main when running contrib-updater generate on any repository, and it is also showing up in #24.

Error: An error occurred while decoding a JSON value:
  Expected value of type 'Array'.

A longer span is available in #24. It looks like something in the codec may be incorrect?

gillchristian commented 3 years ago

@thomashoneyman the issue was a 404 from the GitHub API I pushed a fix in #24 (https://github.com/purescript-contrib/governance/pull/24/commits/ed1f7b5b75fc49fc791f247de6a7d93893005588) but maybe should be part of a separate PR.

JordanMartinez commented 3 years ago

Sorry about that. I didn't come across that problem when tested my code against the purescript repo, nor when I updated all the changelogs last night.

gillchristian commented 3 years ago

No problem. Maybe it is because I am not a contributor? :thinking:

JordanMartinez commented 3 years ago

I'm not sure.