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

refactor: ignore LGTM from author of pull request. #3283

Merged
merged 2 commits into from Jan 2, 2018

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented Jan 2, 2018

  1. ignore LGTM from the author of the pull request.
  2. remove MAINTAINERS file.
  3. remove ignore case in the pattern.

@codecov-io
Copy link

codecov-io commented Jan 2, 2018

Codecov Report

Merging #3283 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3283      +/-   ##
==========================================
- Coverage   34.81%   34.81%   -0.01%     
==========================================
  Files         277      277              
  Lines       40263    40263              
==========================================
- Hits        14018    14016       -2     
- Misses      24184    24186       +2     
  Partials     2061     2061
Impacted Files Coverage Δ
models/repo_list.go 65.62% <0%> (-1.57%) ⬇️

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 274241c...3f452c2. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 2, 2018
@lunny
Copy link
Member

lunny commented Jan 2, 2018

No, no. Why deleted the MAINTAINERS file?

@appleboy
Copy link
Member Author

appleboy commented Jan 2, 2018

@lunny ignore_maintainers_file set as true will ignore parse the maintainers file so we don't need this file anymore.

@appleboy
Copy link
Member Author

appleboy commented Jan 2, 2018

It will load team member from GitHub API but you can't restrict some member of Gitea team to use the L-GTM service.

@lunny
Copy link
Member

lunny commented Jan 2, 2018

@appleboy all members of team Maintainers on Gitea organization could L-GTM.

@tboerger tboerger 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 2, 2018
@lunny lunny removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 2, 2018
@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 2, 2018
@appleboy
Copy link
Member Author

appleboy commented Jan 2, 2018

@lunny Yes, I know but I don't know why we need to keep the MAINTAINERS file? It always calls the GitHub API.

See: https://github.com/go-gitea/lgtm/blob/master/web/hook.go#L55-L74

@tboerger tboerger 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 2, 2018
@lunny
Copy link
Member

lunny commented Jan 2, 2018

@appleboy that's not for lgtm.

@tboerger tboerger 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 2, 2018
@appleboy
Copy link
Member Author

appleboy commented Jan 2, 2018

@lunny revered and change lgtm pattern to (?)LGTM.

@lunny lunny added this to the 1.4.0 milestone Jan 2, 2018
@appleboy
Copy link
Member Author

appleboy commented Jan 2, 2018

@lunny @lafriks merge this PR?

@lunny lunny merged commit 732d31f into go-gitea:master Jan 2, 2018
@appleboy appleboy deleted the patch-1 branch January 2, 2018 14:54
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants