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

Fix branch commit message too long problem #25588

Merged
merged 4 commits into from
Jun 30, 2023

Conversation

lunny
Copy link
Member

@lunny lunny commented Jun 29, 2023

When branch's commit CommitMessage is too long, the column maybe too short.(TEXT 16K for mysql).
This PR will fix it to only store the summary because these message will only show on branch list or possible future search?

@lunny lunny added the type/bug label Jun 29, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 29, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 29, 2023
@lunny lunny added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 29, 2023
@wolfogre
Copy link
Member

I am sure it works, but it's unclear when to use message summary.
image

My suggestion:
main...wolfogre:gitea:bugfix/fix_branch_commit_message_len

Now it's clear:

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 30, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 30, 2023
@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 Jun 30, 2023
@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 Jun 30, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 30, 2023
@lunny lunny merged commit 65acd1e into go-gitea:main Jun 30, 2023
23 checks passed
@GiteaBot GiteaBot added this to the 1.21.0 milestone Jun 30, 2023
@lunny lunny deleted the lunny/fix_branch_commit_message_len branch June 30, 2023 09:03
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 30, 2023
@silverwind
Copy link
Member

@lunny backport?

@@ -77,19 +77,19 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository,
RepoID: repo.ID,
Name: branch,
CommitID: commit.ID.String(),
CommitMessage: commit.CommitMessage,
Copy link
Contributor

@wxiaoguang wxiaoguang Jul 1, 2023

Choose a reason for hiding this comment

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

The meaning of "CommitMessage" in the branch struct has changed now.

If it's in main dev branch so there is still a chance to rename the field.

At least, there should be comment to say that it only contains the message summary but not the full message and why.

@wxiaoguang
Copy link
Contributor

And, I am curious what why it's merged too fast. AFAIK, silverwind also complained that some PRs got merged too fast and suggested to wait for enough time after lgtm/done

@lunny
Copy link
Member Author

lunny commented Jul 1, 2023

@lunny backport?

It cannot be backport because branch is 1.21 only feature

zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 2, 2023
* giteaoffical/main: (89 commits)
  Move some files under repo/setting (go-gitea#25585)
  Following-up improvments for various PRs (go-gitea#25620)
  Set SSH_AUTHORIZED_KEYS_BACKUP to false (go-gitea#25412)
  Fix bug of branches API with tests (go-gitea#25578)
  [skip ci] Updated translations via Crowdin
  Application as a maintainer (go-gitea#25614)
  Adding  branch-name copy  to clipboard branches screen. (go-gitea#25596)
  Use AfterCommitId to get commit for Viewed functionality (go-gitea#25529)
  Fix branch commit message too long problem (go-gitea#25588)
  Restrict `[actions].DEFAULT_ACTIONS_URL` to only `github` or `self` (go-gitea#25581)
  Add API for changing Avatars (go-gitea#25369)
  read-only checkboxes don't appear and don't entirely act the way one might expect (go-gitea#25573)
  Redirect to package after version deletion (go-gitea#25594)
  Update emoji set to Unicode 15 (go-gitea#25595)
  Fix `lint-swagger` action (go-gitea#25593)
  Replace fomantic divider module with our own (go-gitea#25539)
  Add documentation about supported workflow trigger events (go-gitea#25582)
  Sync branches into databases (go-gitea#22743)
  Fix milestones deletion (go-gitea#25583)
  Reduce table padding globally (go-gitea#25568)
  ...

# Conflicts:
#	templates/repo/wiki/revision.tmpl
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants