Skip to content

Conversation

@Zettat123
Copy link
Contributor

This PR modifies the commit message format for squash merge to match GitHub's style.

Screenshots

Before:

ddfd45324aebe5244bab363a807ee915

After:

47bd856598c94d5dedf73148f347e6ec

GitHub:

a72cf5adb41143dc587e3a706b7523e2

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 20, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Nov 20, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 20, 2025
@lunny lunny added this to the 1.26.0 milestone Nov 20, 2025
@lunny lunny added the type/enhancement An improvement of existing functionality label Nov 20, 2025
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 20, 2025
Comment on lines 1262 to 1273
expectedMessage: `* Refactor user service
Implement the login endpoint.
Validate request body.
* Add email notification service
Implements a new email notification module.
- Supports templating
- Supports HTML and plain text modes
- Includes retry logic
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really a good result?

* Refactor user service
...
- Supports templating
* (others...)

Not sure whether GitHub does so. Even if GitHub does so, I don't think GitHub was right or we need to blindly follow GitHub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't quite understand where the problem is. We add * before every commit message and - comes from the commit message content.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example:

commit1:

a
* a
* c

commit2:

x
* y
* z

The current code generates:

* a
* b
* c
* x
* y
* z

It looks like there are 6 commits.

But ideally it should be:

* a
  * b
  * c
* x
  * y
  * z

Just my opinion.

@wxiaoguang
Copy link
Contributor

What's the difference of " Squash merge messages contain information about all commits by default. #35247 " ? Why 35247 seems more complex and changed more code.

@wxiaoguang wxiaoguang marked this pull request as draft November 20, 2025 06:16
@Zettat123
Copy link
Contributor Author

What's the difference of " Squash merge messages contain information about all commits by default. #35247 " ? Why 35247 seems more complex and changed more code.

Gitea already supports including the commit messages of all commits for squash merge in #16134 and this behavior is controlled by the POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES option. It seems that #35247 makes it the default behavior, and it is no longer controlled by the option. I think we should keep this option and only modify the format.

@wxiaoguang wxiaoguang marked this pull request as ready for review November 20, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants