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

Add subscriber for check run event in order to handle rerun request #25

Merged
merged 14 commits into from
Sep 2, 2020

Conversation

@XiongKezhi XiongKezhi requested a review from a team August 13, 2020 16:39
* Get branch name from {@link JSONObject}.
*
* This method will be replaced by {@link CheckRunGHEventSubscriber#getBranchName(GHEventPayload.CheckRun, String)}
* after the release of github--api-plugin 1.116. The github-api has already released 1.116 and make
Copy link
Member

Choose a reason for hiding this comment

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

so do we need this method? seems like it should be available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method is available, but using the object from github-api is better since we don't have to parse the json again, and more safe.

the github-api has released 1.116, may just need to wait two or three days, the github-api-plugin will follow up

Copy link
Member

Choose a reason for hiding this comment

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

kk

@XiongKezhi XiongKezhi marked this pull request as ready for review August 16, 2020 03:51
@XiongKezhi XiongKezhi requested a review from a team August 16, 2020 03:51
@XiongKezhi
Copy link
Contributor Author

The next step: in checks-api, add the "rerun" action in the listener for the main build when completed.

@XiongKezhi XiongKezhi requested a review from a team August 17, 2020 05:25
@XiongKezhi
Copy link
Contributor Author

XiongKezhi commented Aug 17, 2020

I think we have talked about this in community boding but I forgot when starting this PR: I found that when a check fails, a rerun bottom will be automatically added by github like this:
截屏2020-08-17 下午9 03 51

So this is different from what I was thinking before (the action in the checks API, which is added by user request regardless the check resule)
截屏2020-08-17 下午9 08 43

I think it make more sense that we support the the github one instead of the requested one, I'll change to that, nothing different but the 'action' field in the webhook payload.

@XiongKezhi XiongKezhi added the enhancement New feature or request label Sep 2, 2020
@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #25 into master will increase coverage by 1.98%.
The diff coverage is 90.56%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #25      +/-   ##
============================================
+ Coverage     77.33%   79.32%   +1.98%     
- Complexity       96      112      +16     
============================================
  Files             7        8       +1     
  Lines           300      353      +53     
  Branches         33       39       +6     
============================================
+ Hits            232      280      +48     
- Misses           48       53       +5     
  Partials         20       20              
Impacted Files Coverage Δ Complexity Δ
...ugins/checks/github/CheckRunGHEventSubscriber.java 90.56% <90.56%> (ø) 16.00 <16.00> (?)

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 f963454...5eca631. Read the comment docs.

@XiongKezhi XiongKezhi merged commit 321521d into master Sep 2, 2020
@XiongKezhi XiongKezhi deleted the rerun-action branch September 2, 2020 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants