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

feat(STONEINTG-843): add consoleURL plr to github checkrun #651

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

kasemAlem
Copy link
Contributor

We added new field to github endpoint CheckRun and CommitStatus on the creation :

  • CheckRun -> DetailsURL
  • CommitStatus -> TargetURL

For more details see STONEINTEG-843

Maintainers will complete the following section

@kasemAlem kasemAlem changed the title fix(stoneinteg): add support console URL in github chekrun and commitStatus fix(stoneinteg): link console URL in github chekrun and commitStatus Mar 28, 2024
@kasemAlem
Copy link
Contributor Author

/retest

@kasemAlem
Copy link
Contributor Author

/retest required

Copy link

openshift-ci bot commented Mar 28, 2024

@kasemAlem: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test integration-service-e2e

Use /test all to run all jobs.

In response to this:

/retest required

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.

@kasemAlem
Copy link
Contributor Author

/test all

@dirgim
Copy link
Collaborator

dirgim commented Mar 28, 2024

I think that the commit (and also PR) title shouldn't be marked as fix, but as feat since this is an implementation of a regular story and not a bugfix. It should also contain the reference to the STONEINTG-843 story instead of stoneintg so we can easily find the referenced jira item in the future.

Also, the title is currently too long and is failing the gitlint check.

@dirgim
Copy link
Collaborator

dirgim commented Mar 28, 2024

/retest

@kasemAlem kasemAlem changed the title fix(stoneinteg): link console URL in github chekrun and commitStatus feat(stoneinteg): add console Link in github chekrun and commitStatus Apr 4, 2024
@kasemAlem kasemAlem changed the title feat(stoneinteg): add console Link in github chekrun and commitStatus feat(STONEINTG-843): add its plr console url in checkRun and commitStatus Apr 4, 2024
@kasemAlem kasemAlem changed the title feat(STONEINTG-843): add its plr console url in checkRun and commitStatus feat(STONEINTG-843): its plr console url in checkRun and commitStatus Apr 4, 2024
@kasemAlem kasemAlem changed the title feat(STONEINTG-843): its plr console url in checkRun and commitStatus feat(STONEINTG-843): its plr consoleURL in checkRun & commitStatus Apr 4, 2024
@kasemAlem
Copy link
Contributor Author

/retest required

Copy link

openshift-ci bot commented Apr 4, 2024

@kasemAlem: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test integration-service-e2e

Use /test all to run all jobs.

In response to this:

/retest required

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.

@kasemAlem
Copy link
Contributor Author

/test all

.vscode/launch.json Outdated Show resolved Hide resolved
@kasemAlem kasemAlem changed the title feat(STONEINTG-843): its plr consoleURL in checkRun & commitStatus feat(STONEINTG-843): add consoleURL plr to github checkrun Apr 4, 2024
@hongweiliu17
Copy link
Contributor

/test integration-service-e2e

git/github/github.go Outdated Show resolved Hide resolved
git/github/github.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 81.08108% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 72.56%. Comparing base (0128956) to head (2dabb6b).
Report is 11 commits behind head on main.

Files Patch % Lines
status/reporter_github.go 69.23% 2 Missing and 2 partials ⚠️
git/github/github.go 78.57% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #651      +/-   ##
==========================================
- Coverage   72.74%   72.56%   -0.18%     
==========================================
  Files          46       46              
  Lines        4590     4615      +25     
==========================================
+ Hits         3339     3349      +10     
- Misses        900      912      +12     
- Partials      351      354       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@hongweiliu17 hongweiliu17 left a comment

Choose a reason for hiding this comment

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

LGTM

 adding the link of IntegrationTestScenarios plr in console to the
 github cehckrun and commitstatus

Signed-off-by: Kasem Alem <[email protected]>
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@dirgim
Copy link
Collaborator

dirgim commented Apr 16, 2024

/retest

Copy link
Collaborator

@dirgim dirgim left a comment

Choose a reason for hiding this comment

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

LGTM

@kasemAlem kasemAlem merged commit e427b46 into konflux-ci:main Apr 16, 2024
18 checks passed
Summary: summary,
StartTime: detail.StartTime,
CompletionTime: detail.CompletionTime,
TestPipelineRunName: detail.TestPipelineRunName,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like you added this in so that you could call FormatPipelineURL in reporter_github.go. Why not just add a URL parameter to the report and call FormatPipelineURL here?

I ask because this is the approach I'm taking in STONEINTG-842. It works, but whatever we do should be the same so I can switch to your approach if there's some advantage to it.

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

Successfully merging this pull request may close these issues.

5 participants