-
Notifications
You must be signed in to change notification settings - Fork 798
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
fix: prevent duplicate headers from being added (#305) #307
fix: prevent duplicate headers from being added (#305) #307
Conversation
Add a failing test that proves conventional-changelog#305 so that a subsequent commit can fix it.
@@ -5,6 +5,7 @@ const conventionalChangelog = require('conventional-changelog') | |||
const fs = require('fs') | |||
const runLifecycleScript = require('../run-lifecycle-script') | |||
const writeFile = require('../write-file') | |||
const START_OF_LAST_RELEASE_PATTERN = /(^##? (?!Change Log$)|<a name=)/m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I really don't know of a bullet-proof way to do this because the format of where the "old content" begins is really dependent on the templates being used. For example, Angular may use either one #
or two ##
depending on what kind of version it is (patch, etc).
This wasn't an issue in standard-version 4.x. I wonder what the code looks like there for making sure it doesn't keep writing the title over and over. |
@ffxsam I didn't use the package at that version, so I'm not familiar with the headings it made, but it appears from looking at the code that the headings contained Link to code: |
Wait, looks like this was fixed in 5.0.1. I just tested & confirmed. See #292 |
@ffxsam I also confirmed that the fix in 5.0.1 seems to be working. Of course, that change (b684c78) did not include any tests. So, you may still want to consider utilizing at least a portion of my PR here to add some tests so that you don't have a regression. Also, both fixes have the same problem of not actually knowing how to parse a CHANGELOG since how you detect where the "old content" begins depends entirely on the format that's being used. |
@jthomerson @ffxsam I'd honestly like to make the heading configurable (so that folks aren't completely tied to the default one); in the process, perhaps we could also make the regex that finds the last regex configurable -- tldr; I'd love to get this fixed, but maybe we should refactor the logic for putting the header in place a tiny bit. |
@jthomerson thank you for the commit, I'm going to use this for a starting point and am going to make the header configurable 👍 |
Fixes #305