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

feat(parser): support using SHA1 of the commit #385

Merged
merged 3 commits into from
Dec 18, 2023

Conversation

orhun
Copy link
Owner

@orhun orhun commented Dec 14, 2023

Description

See #382

Motivation and Context

This PR makes it possible to use the SHA1 of a commit with commit_parsers.

For example:

# skip a specific commit
commit_parsers = [
    { sha = "f6f2472bdf0bbb5f9fcaf2d72c1fa9f98f772bb2", skip = true }
]

Another example:

# override the group of a specific commit
commit_parsers = [
    { sha = "f6f2472bdf0bbb5f9fcaf2d72c1fa9f98f772bb2", group = "Stuff" }
]

How Has This Been Tested?

Unit/integration tests.
I didn't add fixture tests since it requires more work to find out the SHA1 of the created commit and substitue in expected.md.

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)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@schoetbi
Copy link

schoetbi commented Dec 15, 2023

Awesome that you implemented this feature. It really helps me! What I noted when trying this out is that the order of the commit parsers matter. If I placed the SHA skips at the very beginning, it worked. When I moved them to the last position in the list, it did not filter out these commits, even though the git hashes matched. Is there a reason for this? It seems to me that the parsers are processed in order and the first match is processed only because when I moved the parser for the grouping to the front, the SHA skip did not work. I did not check this in code.

Also, the SHAs should match, even if the casing of the letters is not the same as git delivers it.
These two lines should behave the same

commit_parsers = [
  { sha = "7ff168fbac5bf5dfc772e14e1ff23e62a338aab8", skip = true },
  { sha = "7FF168FBAC5BF5DFC772E14E1FF23E62A338AAB8", skip = true },
]

Since this list might get lengthy over time, it might make sense to add a skiplist file to the configuration. The skiplist file then contains all the SHAs that will be ignored.

@orhun
Copy link
Owner Author

orhun commented Dec 15, 2023

Hey, thanks for testing it!

When I moved them to the last position in the list, it did not filter out these commits, even though the git hashes matched. Is there a reason for this?

It is simply because the commit_parsers are processed in order and if a commit is matched by a parser it can't be parsed by another one. There is a related issue here: #117

Maybe I can note that the order matters in the documentation, or even better, support multiple parsers to be used.

Also, the SHAs should match, even if the casing of the letters is not the same as git delivers it.

Good catch! Should be fixed in d698809

Since this list might get lengthy over time, it might make sense to add a skiplist file to the configuration.

Can you create an issue for discussing this feature? We can potentially have this configuration:

# cliff.toml
[git]
commit_parsers_file = "parsers.toml"
# parsers.toml
commit_parsers = [
    { sha = "f6f2472bdf0bbb5f9fcaf2d72c1fa9f98f772bb2", group = "Stuff" }
]

@orhun orhun merged commit 1039f85 into main Dec 18, 2023
38 of 40 checks passed
@orhun orhun deleted the feat/support_sha_in_commit_parsers branch December 18, 2023 22:55
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

Successfully merging this pull request may close these issues.

2 participants