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

Breaking changes viped from changelog #272

Closed
tunderdomb opened this issue Sep 24, 2021 · 1 comment
Closed

Breaking changes viped from changelog #272

tunderdomb opened this issue Sep 24, 2021 · 1 comment

Comments

@tunderdomb
Copy link

Just want to point out, that for a package that's downloaded tens of millions of times per week, you guys merged a PR (#253) that essentially removed all breaking change warnings from the changelog.
Upgrading dependencies for a project is a tedious task; changelogs help with that. I understand the intent was to improve the changelog, but this clearly removed valuable info.
Please take my feedback as constructive in the hopes that it improves your work on a highly used library in the future.
And please consider reinstating breaking changes into your changelog so that other developers don't have to spend valuable time going through your git history to figure out what footguns they upgraded to.

@silkentrance
Copy link
Collaborator

silkentrance commented Sep 27, 2021

Thank you for pointing this out.

We had a few bug fixes erroneously labelled as breaking changes.

- (Breaking Change) SIGINT handler has been removed.

was replaced by

* (Bug Fix) [#193](https://github.com/raszi/node-tmp/pull/193) Closes [#192](https://github.com/raszi/node-tmp/issues/192): tmp must not exit the process on its own ([@silkentrance](https://github.com/silkentrance))

and similar such cases.

But we also lost some information in the process of switching to lerna-changelog, namely the information that was entered manually and for which no PR exists, e.g.

-  node versions < v8.17.0 are no longer supported. 

and similar such information.

However, we broke the system somewhat when forcing all paths to be relative to the configured tmpdir.

-  template option no longer accepts arbitrary paths. all paths must be relative to os.tmpdir().
-  the template option can point to an absolute path that is located under os.tmpdir().
-  this can now be used in conjunction with the dir option. 
...
-  dir option no longer accepts arbitrary paths. all paths must be relative to os.tmpdir().
-  the dir option can point to an absolute path that is located under os.tmpdir().
...
-  name option no longer accepts arbitrary paths.
-  name must no longer contain a path and will always be made relative to the current os.tmpdir() and the optional dir option.
...
-  fail early if no os.tmpdir() was specified.
-  previous versions of Electron did return undefined when calling os.tmpdir().
-  _getTmpDir() now tries to resolve the path returned by os.tmpdir().
-  now using rimraf for removing directory trees.

These changes are now labelled as either bug fixes or enhancements or internal.

The need for all paths to be relative to the configured tmpdir along with also limitations on the tmp names and paths did raise some issues and we are trying to address that with the future typescript rewrite.

And, we must make sure that we have a PR for every change we make so that it gets labelled correctly and will show up in the automatically generated changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants