feat: notifying users with a message when PR validation fails
- feat: notify users with a message when PR validation fails
Resolves #174
Merge request reports
Activity
added 1 commit
- 9637facc - feat: using proper body structure against github API
@malowe I'm creating this draft even though the code is far from being finished, but this way I could get early feedback from you given the local time difference.
Here I've been implementing GitHub's comment-add API https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#create-a-review-comment-for-a-pull-request
Tomorrow I'll continue with what we discussed on the issue.
241 GithubCommitStatusRequest 242 .builder() 243 .setDescription("Commits in merge group should be previously validated, auto-passing HEAD commit") 246 GithubCommitStatusRequest.builder() 247 .setDescription( 248 "Commits in merge group should be previously validated, auto-passing HEAD commit") 244 249 .setState(GithubCommitStatuses.SUCCESS.toString()) 245 .setContext(webhooksConfig.github().context()) 250 .setContext(webhooksConfig.github().context()).build()); 251 } 252 253 private void commentOnFailure(GithubWebhookRequest request, Set<String> errors) { 254 if (errors.isEmpty()) { 255 return; 256 } 257 StringBuilder sb = new StringBuilder(); changed this line in version 6 of the diff
30 31 @QuarkusTest 32 class GithubHelperTest { 33 34 @Inject 35 GithubHelper helper; 36 37 @RestClient 38 @InjectSpy 39 GithubAPI ghApi; 40 41 @InjectMock 42 ValidationService validationService; 43 44 @Test 45 void testHandleGithubWebhookValidation_WithErrors() { I see what you mean about it not really aligning with testing strategy. We do end to end (e2e) testing typically as it gives us full coverage along with the built in filters and interceptros, but if you want to add additional tests outside of the e2e HTTP testing, that's fine as long as it isn't a direct output of the resource. In this case it is a side effect of a certain use case, and while adding it to our e2e testing would be good for completeness, I don't think it's a deal breaker in this case.
changed this line in version 19 of the diff
238 .updateStatus(jwtHelper.getGhBearerString(request.getInstallation().getId()), apiVersion, 243 ghApi.updateStatus(jwtHelper.getGhBearerString(request.getInstallation().getId()), apiVersion, 239 244 request.getRepository().getOwner().getLogin(), request.getRepository().getName(), 240 245 request.getMergeGroup().getHeadSha(), 241 GithubCommitStatusRequest 242 .builder() 243 .setDescription("Commits in merge group should be previously validated, auto-passing HEAD commit") 246 GithubCommitStatusRequest.builder() 247 .setDescription( 248 "Commits in merge group should be previously validated, auto-passing HEAD commit") 244 249 .setState(GithubCommitStatuses.SUCCESS.toString()) 245 .setContext(webhooksConfig.github().context()) 250 .setContext(webhooksConfig.github().context()).build()); 251 } 252 253 private void commentOnFailure(GithubWebhookRequest request, Set<String> errors) { changed this line in version 6 of the diff
added 1 commit
- a1019488 - feat: using Qute to create a more flexible message template
added 1 commit
- 883e4abb - feat: checking for existing comments before commenting one more time
