You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Config.changelog set to false, still writes CHANGELOG.MD files after exiting prerelease #896
When I exit a prerelease and run yarn changeset version to apply my changesets, I also end up with changelog files being created in all the packages of the monorepo. I am not sure if this happens also without pre, I haven't tested that, as we only do prerelease flows.
Some superficial investigation
From a quick check on the source code it seems the getNewChangelogEntry function correctly checks whether changelogs have been disabled to skip some initial logic, but then proceeds to return a list of packages with changelogs anyway.
Later in the code, the bit of code that processes the list of finalisedRelease entries doesn't check the config either. Instead it validates the presence of the changelog attribute. E.g. changelog && changelog.length > 0.
Unless I am completely off (I only spent a few minutes reading the code), since finalisedRelease is partially based on the output of getNewChangelogEntry, I'd guess that most probably the changelog attribute is always present and filled with something.
It's not a big deal for us, we can run a find and delete the changelogs before committing. That said, I think quite a bit of logic runs to process changelogs and it could be entirely skipped when that setting is set to false.
Proposed solution
Would be really cool if setting changelog to false in the config.json would result in no changelogs being generated at all, just package.json updates.
A quick fix could be to either skip getNewChangelogEntry when config.changelog is false, or double check whether changelogs have been disabled when processing the list of finalisedRelease and skip the changelog logic there.
I'd be very happy to try and fix this up, or test it if necessary.
The same~ message is still in the codebase now and thus it should be adjusted to make it clear that false is also a valid option.
Since this PR only declares the new config shape etc but doesn't really implement any features I've also rechecked the code at its point in time and it seems that back then there was an updateChangelog option to control what you want:
This makes me believe that the original intention to allow false was to, in fact, turn off the changelog generation.
By digging a little bit deeper we can also see that at some point with the new config it was working as intended because the "default" getChangelogFunc was returning an empty string:
Activity
Andarist commentedon Aug 2, 2022
I've checked the PR that introduced
schema.jsonand it seems thatfalsewas always a valid value there:https://github.com/changesets/changesets/pull/132/files#diff-d3013abc54530a5bedc30c2f340c829e5dc49923a70bdf4cc469180bf047cb38R18-R21
Types added as part of this PR also suggest so:
https://github.com/changesets/changesets/pull/132/files#diff-e71f2026e90f1ed0c00f8711eb855f0a8810f33dbf1491f5da9c97cdd513e5b4R64
and even more so the
getNormalisedChangelogOptionfunction from this PR:https://github.com/changesets/changesets/pull/132/files#diff-2eafc75a9015704b121dff9585728616f562e1625da9a53e44a86cacb94ae619R16-R21
The validation error message doesn't specify it as allowed though:
https://github.com/changesets/changesets/pull/132/files#diff-2eafc75a9015704b121dff9585728616f562e1625da9a53e44a86cacb94ae619R48-R54
The same~ message is still in the codebase now and thus it should be adjusted to make it clear that
falseis also a valid option.Since this PR only declares the new config shape etc but doesn't really implement any features I've also rechecked the code at its point in time and it seems that back then there was an
updateChangelogoption to control what you want:changesets/packages/cli/src/commands/bump/index.js
Lines 59 to 68 in 0e81790
This makes me believe that the original intention to allow
falsewas to, in fact, turn off the changelog generation.By digging a little bit deeper we can also see that at some point with the new config it was working as intended because the "default"
getChangelogFuncwas returning an empty string:changesets/packages/apply-release-plan/src/index.ts
Line 130 in 93ca916
and because it stayed used when
config.changelog === falsethen the CHANGELOG files were never getting updated here:changesets/packages/apply-release-plan/src/index.ts
Lines 87 to 90 in 93ca916
So this has accidentally regressed in this PR when
getChangelogFuncbecamegetChangelogFuncs:https://github.com/changesets/changesets/pull/147/files#diff-81efc39f0a283b4d7521917c7d32688cc909b4c27820807c7fc5e554b9ab37c7R134-R137
TLDR: I'm confident to classify this as a bug and I would gladly accept a PR fixing this 😉
sdirosa commentedon Aug 2, 2022
Thanks for the quick reply and in-depth analysis @Andarist! It really helps to see the history of the code and where this regressed.
By the way, the documentation also describes this intention of using
falseas a way of disabling changelog generation:Where it states:
If I can make some time I'll definitely give this a shot.