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

Support linting from the last tag #4110

Merged
merged 2 commits into from
Aug 4, 2024

Conversation

benquarmby
Copy link
Contributor

@benquarmby benquarmby commented Jul 31, 2024

Description

Resolves #2507, which has the full problem statement. Also fixed a bug with the --last option.

In draft mode to get the discussion going. Will open it up for real after more thorough testing. Testing complete (see below) and this is now ready to review.

Motivation and Context

See #2507.

Note that if there are no tags, such as in a brand new repo, commitlint will always pass. The idea is that it should be possible to add commitlint --from-last-tag to CI at the very beginning of a repo and never remove it. There is no need to document special steps, like adding an initial tag.

Usage examples

commitlint --from-last-tag

How Has This Been Tested?

Unit tests and locally against this repo:

yarn run commitlint --from-last-tag --verbose
Output

from-last-tag

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

codesandbox-ci bot commented Jul 31, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@benquarmby benquarmby marked this pull request as ready for review August 2, 2024 17:47
Copy link
Contributor Author

@benquarmby benquarmby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some commentary I hope helps with the review.

Comment on lines +31 to +35
const gitCommandResult = await execa(
'git',
['log', '-1', '--pretty=format:%B'],
{cwd}
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find a bug, fix a bug. Previously using the --last option didn't observe --cwd.

Comment on lines +76 to +86
test('should only read the last commit', async () => {
const cwd: string = await git.bootstrap();

await execa('git', ['commit', '--allow-empty', '-m', 'commit Z'], {cwd});
await execa('git', ['commit', '--allow-empty', '-m', 'commit Y'], {cwd});
await execa('git', ['commit', '--allow-empty', '-m', 'commit X'], {cwd});

const result = await read({cwd, last: true});

expect(result).toEqual(['commit X']);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locked in the --last fix with a test. Previously it was not covered.

Comment on lines +45 to +53
'git',
[
'describe',
'--abbrev=40',
'--always',
'--first-parent',
'--long',
'--tags',
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These git arguments have been battle tested against dozens of repos over several years. Most git output is designed for humans, not for machines (i.e. deserialization), so it takes some work to get stable, deterministic output.

Comment on lines +62 to +65
// Description will be in the format: <tag>-<count>-g<hash>
// Example: v3.2.0-11-g9057371a52adaae5180d93fe4d0bb808d874b9fb
// Minus zero based (1), dash (1), "g" prefix (1), hash (40) = -43
const tagSlice = stdout.lastIndexOf('-', stdout.length - 43);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a few ways to skin this cat. The complexity comes from the fact that tags can themselves contain the dash - character (i.e. v1.2.3-alpha.0), and git doesn't escape anything. Need to work backwards from the last dash we can be sure was produced by git.

docs/reference/cli.md Show resolved Hide resolved
@escapedcat
Copy link
Member

Thanks for the comments!

@escapedcat
Copy link
Member

Once this is rebased it can be merged and a release can be done. Might take a couple of days.

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

Successfully merging this pull request may close these issues.

Support --from-last-tag flag
2 participants