Skip to content
This repository has been archived by the owner on May 24, 2021. It is now read-only.

feat: support consider lgtm-like comment as review #44

Closed
wants to merge 3 commits into from

Conversation

Mini256
Copy link
Member

@Mini256 Mini256 commented Feb 2, 2021

CheckedList

  • feat: support consider lgtm-like comment as review
  • update publish workflow

Related

close #38

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 2, 2021
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #44 (b2b0dcd) into main (5e7fd8d) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   88.62%   88.74%   +0.11%     
==========================================
  Files          14       14              
  Lines         378      382       +4     
  Branches       55       57       +2     
==========================================
+ Hits          335      339       +4     
  Misses         29       29              
  Partials       14       14              
Impacted Files Coverage Δ
src/services/PullService.ts 91.26% <100.00%> (+0.35%) ⬆️

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 5e7fd8d...b6f6d5e. Read the comment docs.

@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 2, 2021
@Rustin170506
Copy link
Contributor

@Mini256 Why we need this?

@Mini256
Copy link
Member Author

Mini256 commented Feb 2, 2021

@Mini256 Why we need this?

Because the user may only enter the comment of /lgtm, but he actually completed the review process.

@Mini256
Copy link
Member Author

Mini256 commented Feb 2, 2021

It‘s a TODO item in the github-tool source code.

image

@Rustin170506
Copy link
Contributor

It‘s a TODO item in the github-tool source code.

image

Maybe we shouldn't count that way at the moment. Because some people's /lgtm may be invalid.

@Rustin170506
Copy link
Contributor

/hold

@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 2, 2021
@Rustin170506
Copy link
Contributor

Actually, I don't know what the point of this record is?It's pretty much useless as it stands and would break the ability to persist GitHub reviews.

@Mini256
Copy link
Member Author

Mini256 commented Feb 2, 2021

Actually, I don't know what the point of this record is?It's pretty much useless as it stands and would break the ability to persist GitHub reviews.

The last review time will not affect the persist of GitHub reviews.

@Rustin170506
Copy link
Contributor

The last review time will not affect the persist of GitHub reviews.

Got it. But there still seems to be an invalid /lgtm problem?

@Mini256
Copy link
Member Author

Mini256 commented Feb 3, 2021

Got it. But there still seems to be an invalid /lgtm problem?

Indeed.

@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2021
@ti-chi-bot
Copy link
Member

@Mini256: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Mini256 Mini256 closed this Mar 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the comment containing lgtm as a review to update the last review time of PR.
3 participants