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

4 participants
@appleboy
Copy link
Member

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

This comment has been minimized.

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 label Jan 2, 2018

@lunny

This comment has been minimized.

Copy link
Member

lunny commented Jan 2, 2018

No, no. Why deleted the MAINTAINERS file?

@appleboy

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

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 and removed lgtm/need 2 labels Jan 2, 2018

@lunny lunny removed the lgtm/need 1 label Jan 2, 2018

@tboerger tboerger added the lgtm/need 2 label Jan 2, 2018

@appleboy

This comment has been minimized.

Copy link
Member

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 and removed lgtm/need 2 labels Jan 2, 2018

@lunny

This comment has been minimized.

Copy link
Member

lunny commented Jan 2, 2018

@appleboy that's not for lgtm.

@tboerger tboerger added lgtm/done and removed lgtm/need 1 labels Jan 2, 2018

@appleboy appleboy force-pushed the appleboy:patch-1 branch from 240f5b9 to 697a916 Jan 2, 2018

@appleboy

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

appleboy commented Jan 2, 2018

@lunny @lafriks merge this PR?

@lunny lunny merged commit 732d31f into go-gitea:master Jan 2, 2018

3 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@appleboy appleboy deleted the appleboy:patch-1 branch Jan 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment