-
Notifications
You must be signed in to change notification settings - Fork 445
Description
Thank you to the creators of release-please
for your hard work and dedication to the project.
I wanted to bring to your attention an issue with the current implementation of the parsing logic in src/commit.ts
. It appears that the logic does not handle multiple conventional commits within a single squash merge correctly. The implementation seems to rely heavily on specific formatting and structure that does not fully align with the Conventional Commits specification.
For example, problems arise when commit messages include bullet points, empty lines, or varying indentations, as demonstrated in the experiment examples provided. Additionally, the documentation does not mention the requirement for body messages to be indented by two spaces, as seen in some examples in the repository's README. This discrepancy suggests that the current logic may not fully adhere to the specification, which the repository aims to support.
Here is the conventional commit format which I believe the examples below all fit in.
<type>[optional scope]: <description>
[optional body]
[optional footer(s)]
Here are some examples from the experiments I made
Slightly modified example of the example in the README - Parsed correctly, however there needs to be a new line between body and title
feat: adds v4 UUID to crypto
This adds support for v4 UUIDs to the library.
fix(utils): unicode no longer throws exception
- some more stuff
feat(utils): update encode to support unicode
- does stuff
- more stuff
Same example except this time with bullets are at the body (Parsed Incorrectly)
feat: adds v4 UUID to crypto
This adds support for v4 UUIDs to the library.
fix(utils): unicode no longer throws exception
- some more stuff
feat(utils): update encode to support unicode
- does stuff
- more stuff
Same example with an empty line between the description and the body (Parsed Incorrectly - only the first commit is treated as a conventional commit)
feat: adds v4 UUID to crypto
This adds support for v4 UUIDs to the library.
fix(utils): unicode no longer throws exception
- some more stuff
feat(utils): update encode to support unicode
- does stuff
- more stuff
Multiple commits with a simple body (Parsed Incorrectly - only the first commit is treated as a conventional commit)
feat: adds v4 UUID to crypto
This adds support for v4 UUIDs to the library.
fix(utils): unicode no longer throws exception
some more stuff
feat(utils): update encode to support unicode
does stuff
more stuff
Additional Details
- I found out that the
BEGIN_NESTED_COMMIT
andEND_NESTED_COMMIT
could be used to customize the nested commits, however I think it would be awesome if release please can handle these automatically. - I feel like the
splitMessages
function could be modified to support the aforementioned commits and improve the parsing. A simple change like below supports the commit types I mentioned above.(It brakes only 5 tests)
function splitMessages(message: string): string[] {
const parts = message.split('BEGIN_NESTED_COMMIT');
const messages = [parts.shift()!];
for (const part of parts) {
const [newMessage, ...rest] = part.split('END_NESTED_COMMIT');
messages.push(newMessage);
messages[0] = messages[0] + rest.join('END_NESTED_COMMIT');
}
// Split the first message into conventional commits
const conventionalCommits = messages[0].split(/\n(?=(?:feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.*?\))?: )/);
return [...conventionalCommits, ...messages.slice(1)];
}
Environment details
- OS: macos sonoma 14.5
- Node.js version: v22.2.0
- npm version: 10.7.0
release-please
version: 16.12.0
Steps to reproduce
- Create a new txt file from the examples above in the
test/fixtures/commit-messages/
directory - Create a new test in
test/commits.ts
with the following content
it('test custom commits', async () => {
const commits = [buildCommitFromFixture('name-of-the-fixture')];
const conventionalCommits = parseConventionalCommits(commits);
console.log('Conventional Commits:');
console.log(JSON.stringify(conventionalCommits, null, 2));
expect(conventionalCommits).lengthOf(1);
expect(conventionalCommits[0].breaking).to.be.true;
expect(conventionalCommits[0].message).not.include('I should be removed');
});
3. Run `npm test` > out.txt
5. Search the `out.txt` file for "Conventional Commits:" to inspect the conventionalCommits array.