Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: switch to ESM, fix output colourising, dedupe more code w/ branch-diff #121

Merged
merged 3 commits into from
Jan 17, 2022

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Dec 3, 2021

  • upgrade all deps, including ESM-only deps
  • --simple is now default output, including colourising, with --markdown being an opt-in output format (that doesn't colourise).
  • move process + print logic to separate module for exporting for simplifying branch-diff
  • add --sha and --reverse options from branch-diff to dedupe some processing code (will open an PR over there that pulls this in)

The biggest change is that second item, the default output is nice for the terminal but if you want it for changelogs then you'll need to --md to get it and you won't have colourisation for that. The new markdown formatter and ansi don't mix. So this change will require some @nodejs/releasers input.

Change set is probably a bit for a full review, but if you're interested then maybe clone it and npm link it to try it out.

Fixes: #120
Closes: #107
Closes: #119

@rvagg rvagg force-pushed the rvagg/esm-upgr branch 2 times, most recently from 613718e to 286d2a1 Compare December 3, 2021 07:40
rvagg added a commit to nodejs/branch-diff that referenced this pull request Dec 3, 2021
@Trott
Copy link
Member

Trott commented Dec 3, 2021

LGTM but I'm not a releaser so I'm not sure if/how this impacts their process....

@richardlau
Copy link
Member

The biggest change is that second item, the default output is nice for the terminal but if you want it for changelogs then you'll need to --md to get it

I think switching the default would mean we'd have to update https://github.com/nodejs/node-core-utils/blob/e7a95a4ec4b166b9311c673f1d4617da4a13d2bc/lib/prepare_release.js#L291-L303 when we update https://github.com/nodejs/node-core-utils/blob/d1c52e669439dbe22bc902cfa5d767328b91b06b/package.json#L36. I haven't really used that aspect of node-core-utils much (normally I run changelog-maker and copy/paste into the changelog being updated).

Change set is probably a bit for a full review, but if you're interested then maybe clone it and npm link it to try it out.

Lazy way to try it out

npx nodejs/changelog-maker#rvagg/esm-upgr ...

…nch-diff

* upgrade all deps, including ESM-only deps
* --simple is now default output, including colourising, with
  --markdown being an opt-in output format.
* move process + print logic to separate module for exporting
  for simplifying branch-diff
* add --sha and --reverse options from branch-diff to dedupe some
  processing code

Fixes: #120
Closes: #107
Closes: #119
@rvagg
Copy link
Member Author

rvagg commented Jan 17, 2022

Whoa this didn't get merged, I forgot all about it.

@nodejs/releasers heads-up this is going out as a 3.0.0 and you'll need a --md to generate changelogs now, but I'm sure you'll notice it coming out wrong if you don't update your workflow - unlikely to slip into a release because of the markdown linter in nodejs/node.

@rvagg rvagg merged commit 1462378 into main Jan 17, 2022
@rvagg rvagg deleted the rvagg/esm-upgr branch January 17, 2022 05:07
@github-actions
Copy link

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

rvagg added a commit to nodejs/branch-diff that referenced this pull request Jan 18, 2022
rvagg added a commit to nodejs/branch-diff that referenced this pull request Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coloring is broken
3 participants