Skip to content

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented Jan 6, 2020

Add Reviewed-on in the commit message.

Screenshot:

21312321___5__·06470f8899-2132131-_Gitea__Git_with_a_cup_of_tea

@appleboy appleboy marked this pull request as ready for review January 6, 2020 14:52
@appleboy appleboy added the type/enhancement An improvement of existing functionality label Jan 6, 2020
@codecov-io
Copy link

codecov-io commented Jan 6, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@ccf9988). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #9623 +/- ## ========================================= Coverage ? 42.18% ========================================= Files ? 583 Lines ? 77253 Branches ? 0 ========================================= Hits ? 32589 Misses ? 40656 Partials ? 4008
Impacted Files Coverage Δ
modules/webhook/telegram.go 3.97% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccf9988...d1a66ca. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 6, 2020
@6543
Copy link
Member

6543 commented Jan 6, 2020

Dont know if this is usefull - the pr number is already in the commit title

@sapk
Copy link
Member

sapk commented Jan 6, 2020

@zeripath
Copy link
Contributor

zeripath commented Jan 6, 2020

I think I'd prefer Referenced-PR: or something like that.

The trouble with Reviewed-On: is that it makes me think of a date, a particular commit or computer architecture not the PR number and so the PR number comes as a bit of a shock.

@lunny lunny added this to the 1.12.0 milestone Jan 7, 2020
@appleboy
Copy link
Member Author

appleboy commented Jan 7, 2020

I take golang/go@edf3ec9 as reference

@zeripath
Copy link
Contributor

zeripath commented Jan 7, 2020

Ah I had no idea they were using that. Have you been able to find a document that has all their tags?

OK, I wonder if we should be putting the full url in there then or at least the reponame/username.


which is what you're doing...

@zeripath
Copy link
Contributor

zeripath commented Jan 7, 2020

Now sticking the full URL in could unintentionally expose private giteas.

I guess this needs probably needs a config value. (yet another)

I would guess options include: full URL, owner/reponame#1, owner/reponame!1, #1, !1, none?

@lunny
Copy link
Member

lunny commented Jan 8, 2020

@zeripath If you can visit the codes or the pull, then you have know the private repository URL before the commit message tell you. I don't think it's a problem.

If we put owner/reponame#1, it will only be a link on gitea but not git commands or other git tools.

@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 Jan 8, 2020
@guillep2k
Copy link
Member

@zeripath If you can visit the codes or the pull, then you have know the private repository URL before the commit message tell you. I don't think it's a problem.

Unless it's a public mirror of a private repo from another instance.

A full URL would not survive well a site migration, but a relative one will only work locally which is a boomer as well.

@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 Jan 8, 2020
@sapk
Copy link
Member

sapk commented Jan 8, 2020

It could be configurable on repository level (furthermore maybe add a commit template ?) but in the meantime the user can simply remove the text ?

@appleboy
Copy link
Member Author

appleboy commented Jan 8, 2020

@sapk Yes. Users can simply remove the text before merging the PR.

8-Update__README_md_-2132131-_Gitea__Git_with_a_cup_of_tea

@zeripath
Copy link
Contributor

zeripath commented Jan 9, 2020

Make lg-tm work

@zeripath zeripath merged commit 0752043 into go-gitea:master Jan 9, 2020
@appleboy appleboy deleted the pr branch January 9, 2020 15:44
@danwilliams
Copy link

@appleboy @sapk @zeripath is there an issue that covers making this configurable as a system-wide default? I'd like to keep my eye on it if so, as it is tedious to manually remove the text for every merge.

(I could not find one in a few minutes of searching.)

@sapk
Copy link
Member

sapk commented Jan 18, 2020

I don't think so but it could maybe be attached to #9823 to provide commit template for commit (update and merge) globally, by org/user or repo.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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. type/enhancement An improvement of existing functionality

9 participants