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: go template function uniqueOlderCommits/uniqueNewerCommits is available #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikedam
Copy link

@ikedam ikedam commented Sep 18, 2022

  • They allow to remove duplicated log entries.

Closes #29

What does this do / why do we need it?

  • Introduce new go template custom functions uniqueOlderCommits and uniqueNewerCommits. They removes duplicated commits / log entries. "Duplicated" is evaluated with fields specified. For example, if your changelog entries consists of .Scope and .Subject, (uniqueOlderCommits .Commits "Scope" "Subject") is expected.
  • There are cases log entries are duplicated. See Expect to filter duplicate commits! #29. We had to write really complicated go templates to remove those duplication. These new functions let us do that easily.

We can rewrite .chglog/CHANGELOG.tpl.md in git-chglog like this:

diff --git a/.chglog/CHANGELOG.tpl.md b/.chglog/CHANGELOG.tpl.md
index 5683d03..db302fe 100755
--- a/.chglog/CHANGELOG.tpl.md
+++ b/.chglog/CHANGELOG.tpl.md
@@ -5,7 +5,7 @@
 {{ if .Unreleased.CommitGroups -}}
 {{ range .Unreleased.CommitGroups -}}
 ### {{ .Title }}
-{{ range .Commits -}}
+{{ range (uniqueOlderCommits .Commits "Scope" "Subject") -}}
 - {{ if .Scope }}**{{ .Scope }}:** {{ end }}{{ .Subject }}
 {{ end }}
 {{ end -}}
@@ -17,7 +17,7 @@
 ## {{ if .Tag.Previous }}[{{ .Tag.Name }}]{{ else }}{{ .Tag.Name }}{{ end }} - {{ datetime "2006-01-02" .Tag.Date }}
 {{ range .CommitGroups -}}
 ### {{ .Title }}
-{{ range .Commits -}}
+{{ range (uniqueOlderCommits .Commits "Scope" "Subject") -}}
 - {{ if .Scope }}**{{ .Scope }}:** {{ end }}{{ .Subject }}
 {{ end }}
 {{ end -}}

How this PR fixes the problem?

  • The underlying problem of Expect to filter duplicate commits! #29 is:
    • We have to filter Commit to remove duplication.
    • But it's really difficult to handle structured data with go template: Sprig doesn't work for structured data. Go template doesn't provide mechanisms like lambda functions.
    • What we want to do is: Detect duplication only with specific fields in Commit. For example, "Scope" and "Subject". Or it may be JiraIssueID (for the case only one entry for one ticket).
  • I believe uniqueOlderCommits and uniqueNewerCommits resolves that.

What should your reviewer look out for in this PR?

  • Let me know if better function names.
  • Please propose better descriptions in README.md. This feature is too complicated for my English.

Check lists

  • Test passed
  • Coding style (indentation, etc)

Additional Comments (if any)

  • Older and Newer means the commit order. I believe we also need fix: the sort order of commits is unstable #210 to have this feature fully functioned.
  • This code isn't good in performance (O(n^2)). But this doesn't need so much performance and I believe it's better to keep easy to read and easy to maintenance.

Which issue(s) does this PR fix?

fixes #29

@mavogel mavogel added this to the v0.16.0 milestone Jan 21, 2023
@mavogel mavogel self-requested a review January 21, 2023 07:59
@mavogel mavogel mentioned this pull request Jan 21, 2023
2 tasks
@mavogel
Copy link
Member

mavogel commented Jan 22, 2023

@ikedam thank you for the PR. Could you rebase it onto the latest master?

@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

@mavogel Sounds strange. I can't get what you want.
This doesn't conflict with the current master and there looks no need to rebase. Why do you want this rebased?

@mavogel
Copy link
Member

mavogel commented Jan 24, 2023

hi @ikedam to see of the tests and linting still pass with the updated versions of the master, for e.g. golang, golangci-lint, etc. :)

@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

@mavogel
I believe that should be verified with CI.
Problems on CI? Somehow lint and tests doesn't seem run, but I can't see what happen. Rebase is required to run CI?

@mavogel
Copy link
Member

mavogel commented Jan 24, 2023

I believe that should be verified with CI.
Exactly yes, in this branch. However, weird, that CI is not triggered

@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

Let's try reopen.

@ikedam ikedam closed this Jan 24, 2023
@ikedam ikedam reopened this Jan 24, 2023
@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

Hmm... not triggered. OK. I'll rebase this.

…vailable

* They allow to remove duplicated log entries.

Closes git-chglog#29
@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

https://github.com/git-chglog/git-chglog/actions/runs/3995523177

This workflow is awaiting approval from a maintainer in #211

@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

But the message changed to

1 workflow awaiting approval First-time contributors need a maintainer to approve running workflows. Learn more.

and differs from the one before rebasing. Something wrong looked happen inside github.

Anyway, rebase completed.

@mavogel
Copy link
Member

mavogel commented Jan 24, 2023

ah now I got the approve run button and pushed it :)

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3995523177

  • 82 of 87 (94.25%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 77.395%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chglog.go 66 71 92.96%
Totals Coverage Status
Change from base Build 3993620083: 0.6%
Covered Lines: 1931
Relevant Lines: 2495

💛 - Coveralls

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.

Expect to filter duplicate commits!
3 participants