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

publishChecks step: the conclusion should be none when status is not completed #72

Closed
XiongKezhi opened this issue Jan 26, 2021 · 13 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@XiongKezhi
Copy link
Contributor

Currently, the publishChecks step has a default success value for the conclusion, however, it should reset the conclusion to be none when the status is queued or in_progress. Otherwise, users may have to explicitly set the conclusion to be none since, like GitHub, it will automatically set the status to be completed when conclusion is provided.

See the context: #70 (comment)

@XiongKezhi XiongKezhi added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Jan 26, 2021
@ThusithaDJ
Copy link
Contributor

Hi @XiongKezhi, If this fix is not super urgent, can I work on this?

@XiongKezhi
Copy link
Contributor Author

go ahead! thanks!

@ThusithaDJ
Copy link
Contributor

@XiongKezhi, is this issue fixed, or am I checking at the wrong place?

getChecksName(job).ifPresent(checksName -> publish(ChecksPublisherFactory.fromJob(job, TaskListener.NULL),
ChecksStatus.QUEUED, ChecksConclusion.NONE, checksName, null));

getChecksName(run).ifPresent(checksName -> publish(ChecksPublisherFactory.fromRun(run, listener),
ChecksStatus.IN_PROGRESS, ChecksConclusion.NONE, checksName, null));

@KalleOlaviNiemitalo
Copy link

BuildStatusChecksPublisher is the wrong place. You need PublishChecksStep.

@ThusithaDJ
Copy link
Contributor

Thanks, @KalleOlaviNiemitalo. To be clear, I shouldn't change the conclusion variable's default value but need to add a condition to check whether status is queued or in_progress, right?

@KalleOlaviNiemitalo
Copy link

I don't really know what is the best way to implement this. Anyway, please test that the pipeline snippet generator behaves in a sensible way after your changes.

@XiongKezhi
Copy link
Contributor Author

Thanks, @KalleOlaviNiemitalo. To be clear, I shouldn't change the conclusion variable's default value but need to add a condition to check whether status is queued or in_progress, right?

yes, you should add a condition to check the status when setting the conclusion.

@ThusithaDJ
Copy link
Contributor

Ok sure. Thanks a lot, both of you.

@ThusithaDJ
Copy link
Contributor

@XiongKezhi, could you please let me know if you have any guide to follow on setting up the plugin for local testing? I referred to the Consumers Guide and Implementation Guide, but it's a bit unclear for me. It would be better if you can help me with this.

@XiongKezhi
Copy link
Contributor Author

XiongKezhi commented Feb 8, 2021

@ThusithaDJ Sorry, those are not guides for users, they are for other plugin developers.

So when you are trying to test this plugin locally, first you need to set up a Jenkins instance for remote debugging. If you are using a Jenkins war file, startup it with the java options like (for more, see here):

-Xdebug -Xrunjdwp:transport=dt_socket,address=127.0.0.1:8000,server=y,suspend=n

Then you can set up your remote debugger to attach to it.

Finally, you need a Jenkins project to run the code of this plugin:

  1. Set up the GitHub app credentials as in this guide
  2. Use the GitHub app credentials you added to Jenkins in the last step in one of your projects. I have a multi-branch project set up like this (you should use the repository that your GitHub app is installed on):

截屏2021-02-09 上午12 10 53

And since you are going to modify the publishChecks step, so you should have a repository with a Jenkins pipeline calling publishChecks.

@timja
Copy link
Member

timja commented Feb 8, 2021

personally I just change the dependency in the github checks repo to a SNAPSHOT dependency, build this project with mvn install -P quick-build and then run the github checks project with mvn hpi:run using the debug option in my IDE

or just using unit tests for some changes where I don't actually need GitHub

@ThusithaDJ
Copy link
Contributor

Hi @XiongKezhi, I have pushed some changes, please check that and let me know your thoughts.
#82

@XiongKezhi
Copy link
Contributor Author

fixed by #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants