build: do not drop commits with malformed messages in the release note

[wookayin]

Problem: Some commits that do not follow the conventional commits format
are not included in the release noted generated by git cliff.
An example: [Backport release-0.9] fix(build): ...

While CI enforces "lintcommit" checking, such malformed commits might
have been merged via merging "backport to a release branch" through
github, where (accidentally) the commit message gets rewritten and no
longer follows the conventional commits format.

Solution:

• Strip the [Backport release-..] prefix while pre-processing commits,
  because this is a common error case in our current release process
  (applies to the commits that were already merged).

• Set filter_commits = false and filter_unconventional = false, i.e.
  do not filter out the commits whose message do correctly follow the
  conventional commits. This will make mistakes and errors at least
  recognizable while generating a release note during release
  (which should, however, be manually fixed by the release manager).
  NOTE: This changes will also result in new "vim-patch" section.

• Include "ci" groups which were previously missing.

[wookayin]

Changes for release note v0.9.4..v0.9.5 resulting from this PR:

--- CHANGELOG.md
+++ changelog2.md
@@ -8,6 +8,7 @@
 - Dont create data dir if it's a broken symlink
 - Make InspectTree handle nested injection
 - Remove nested for_each_tree in TSTreeView (#26331)
+- Vim.treesitter.get_node() now correctly takes opts.lang (#26382)
 - **api**: Use a conditional stack for nvim_cmd
 - **api**: Don't set coladd of mark
 - **change**: Update fold after on_bytes
@@ -17,6 +18,7 @@
 - **highlight**: Apply 'winblend' to float border
 - **inccommand**: Save and restore '[ and '] marks
 - **inccommand**: Don't crash with "split" and 'n' flag
+- **log**: Increase size of buffer for nvim instance name (#26450)
 - **lsp**: Handle NUL bytes in popup text
 - **lua**: Correct return value for on_key with no arguments
 - **lua**: Crash in nlua_error
@@ -62,6 +64,10 @@
 - **starting.txt**: Correct step number
 
 
+### Features
+- **highlight**: Allow hyphens (-) in highlight group names (#25661)
+
+
 ### Performance
 - Remove redundant strlen in skipwhite
 
@@ -77,4 +83,17 @@
 - **terminal/channel_spec**: Fix flakiness
 
 
+### Vim-patch
+- 9.0.0064: confusing error when using "q:" in command line window
+- 9.0.0218: reading before the start of the line
+- 9.0.0249: no test for what 9.0.0234 fixes
+- 9.0.0490: using freed memory with cmdwin and BufEnter autocmd
+- 9.0.0492: cmdwin test fails on MS-Windows
+- 9.0.0598: using negative array index with negative width window
+- 9.0.2151: 'breakindent' is not drawn after diff filler lines
+- Ff0baca86523
+- 9.0.2159: screenpos() may crash with neg. column
+- 9.0.2178: reg_executing() wrong for :normal with range
+
+
 <!-- generated by git-cliff -->

git cliff --config scripts/cliff.toml v0.9.4..v0.9.5

(Note: the vim-patch section can be manually clipped by dev who is in charge of release, unless we automate everything)

[bfredl]

It is a simple workaround after the fact for release-0.9 but I'm not sure I like the precedent it sets (and thus, adding also to the master branch like this PR does). The risk is that we would just pile on more exceptions, instead of making sure to actually follow the intentional format in the first place.

So, specifically about backports. If there is a general expectation that PR titles of autogenerated PR:s (such as backport PR:s) are good to use, then I think we should not prepend [Backport release-0.9] to such PR titles in the first place, and instead make them standout in some other way (like screaming label colors). but such PR:s get merged very quickly, so there might be no problem with cluttering the open PR view in the first place. Even if PR titles are not validated in general I understand the expectation that auto-generated messages are good, because why would we have generated a bad message and not a good message

[wookayin]

... instead of making sure to actually follow the intentional format in the first place.

Yes! We should make all the commits merged to HEAD (master) and backport release branches follow the correct commit message format -- as much as possible. However, unless everything is automated and strictly enforced, there still might be some exceptions or human mistakes happen. Devs often make use of Github's feature to "edit the commit message" while merging PRs through web UI (to correct typos or fix commit messages at ease), I'm a bit against too-strict enforcement. Nevertheless, we can consider further improvements to the PR backport automation (e.g. do not have the "[backport release]" prefix, but making it as a suffix, etc.).

A minor remark:

Some other kind of exceptions --- #26800 (by @zeertzjq) also draws my attention because it cherry-picks two commits from deps/libtermkey that violate the intended commit message format. It's debatable whether we want these two commits ( b4ef913, ffe96c6) include in the release note, and whether these would be manually edited for the release note during the release process. However, I think being aware of some malformed commit messages would be still beneficial because otherwise git cliff will not generate any warnings or notifications.

git cliff --config scripts/cliff.toml HEAD~5..HEAD

# Changelog

All notable changes to this project will be documented in this file.

## [unreleased]

### (unclassified; TODO: review manually)
- Ignore key_mouse unless it is exactly \e[M because some terminfos relate to different encoding modes (thanks Ninji)
- Handle mouse buttons 6/7 (often used for horizontal scrolling)


### Bug Fixes
- **build**: Do not drop commits with malformed messages in release note


### Refactor
- **tui**: Remove code that is no longer necessary


### Testing
- **functional**: Remove faulty pending check

[bfredl]

I'm a bit against too-strict enforcement.

I am as well, but this means we need to accept that generated and edited release notes are going to contain some errors and omissions as well. Getting rid of systematic sources of errors (i e unfortunate choices in automated scripts) is going to eliminate a lot of unnecessary omissions. Then we can reassess the situation after the next release:)

We can make the process of release notes more open, like some other projects. I e make the notes available as a draft for others to peek at and suggest changes. Although this is going to make the release process more involved. It might be something to consider for feature releases at some point (but not for patch releases, the less extra steps the better).

[justinmk]

We can make the process of release notes more open, like some other projects. I e make the notes available as a draft for others to peek at and suggest changes.

Based on past newsletter PRs, I don't expect much non-core feedback on such a PR.

The risk is that we would just pile on more exceptions, instead of making sure to actually follow the intentional format in the first place.

100%. The purpose of cliff + conventional commits is to reduce the work involved in producing release notes. filter_unconventional = false seems to defeat much of that.

For maintenance releases, can't we just link to the commit history? Changelog isn't as important there.

[wookayin]

Thanks for your comment! Let me point out again that the whole point of this is to prevent commits or release note items from being excluded from the release note. Automation and less manual jobs are of course good, but I think we should aim for correctness too.

The purpose of cliff + conventional commits is to reduce the work involved in producing release notes.

If we want full automation while ensuring correctness (i.e., not unawarely missing any commits), we probably would need have better enforcement. Because of many possible reasons and the current limitation of Github UI/UX, conventional commits are not strictly enforced; (1) a maintainer can change title (on squashing merge) even if lintcommit has passed, and might make some mistakes; (2) commits in PR passed CI but the "title of a PR" had a mistake so the merged commit ended up violating conventional-commit (e.g. 4ee656e and #26807); to name a few. Indeed, for upcoming v0.10 release (between v0.9.0..HEAD), there are already a few commits that violate the commit message format. However, we probably don't want to make the process of merging PR unnecessarily complicated. Errors can happen and are often fine.

If we want to get rid of any manual process in writing the git-cliff-generated release note, we could just have a "uncategorized" or "others" section collecting all the unclassified (incorrectly-formatted) commits instead of manually editing the release note (which, however, shouldn't take too much time, to delete or move around only a few lines).

[justinmk]

we could just have a "uncategorized" or "others" section collecting all the unclassified (incorrectly-formatted) commits

I guess that's ok. That's this part, right?:

{ message = "^.*", group = "(unclassified; TODO: review manually)"},

So that means the release manager can easily delete that whole section, and then it's equivalent to before?

[justinmk]

is this change actually needed? wouldn't the message = "^.*" commit parser match everything?

if this is still needed, let's move it near filter_unconventional since they seem related.

[wookayin]

So that means the release manager can easily delete that whole section, and then it's equivalent to before?

Yes, exactly. I've made a few additional changes so now the "uncategorized" section would come at the end. The new sections are vim patches (not sure we want to include this or not because it's quite messy and looooong) and this uncategorized section; if one removes these two these should be equivalent (except for other changes regarding "[backport release]").

At the moment, I can see around 5 minor commits as well as version.c: Update commits (git cliff --config scripts/cliff.toml v0.9.0..HEAD). BTW there is another issue as a side effect but hope this can be addressed by upstream (git-cliff) fix.

[justinmk]

would it be more reliable to filter merge commits like this: https://git-cliff.org/docs/tips-and-tricks#filter-merge-commits

i.e. change line 17 to :

{% for group, commits in commits | filter(attribute="merge_commit", value=false) | group_by(attribute="group") %}

[wookayin]

Great suggestion. Will do that

[justinmk]

Suggested change

	{ message = "^([^)\r\n]+):", group = "[unclassified; TODO: review manually]", scope = "$1" },
	{ message = "^([^)\r\n]+):", group = "unclassified; review manually, update cliff.toml if necessary", scope = "$1" },

[dundargoc]

This PR will remove the [Backport release-0.9] from being created in pull request titles, so we don't need to explicitly exclude it anymore.

This PR makes the vim-patches.yml follow conventional commits so we no longer have "version.c:".

I think all our automated solutions should use conventional commits after these.

[dundargoc]

Superseded by #28595.

I changed the strategy from including non-conventional commits to just yeeting them as I don't think we have any automated solutions left that create non-conventional commits.