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-524): Report result of handling ITS to github in a new controller #296

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

hongweiliu17
Copy link
Contributor

@hongweiliu17 hongweiliu17 commented Sep 1, 2023

  • Create function to get credential from snapshot
  • Create function to report for result of handle ITSes for snapshot
  • The old EnsureStatusReported() in integration pipeline controller is kept since we don't know yet how to get the test result for integration pipeline runs after PLNSRVCE-1295
  • The controller will get all checkRuns and then compare with the checkRun to be created/updated to if the action is needed before performing the action
  • The controller will get all commitStatuses and then compare with the commitStatus to be created to see if the action is needed before performing the action
  • The new report won't include the testResult and we can do it when we need to remove the current report in integration PLR adapter after PLNSRVCE-1295 is implemented.

Checkrun examples in github https://github.com/hongliuorg/devfile-sample-go-basic/pull/3/checks?check_run_id=16739942606

Maintainers will complete the following section

  • Commit messages are descriptive enough (hints)
  • Code coverage from testing does not decrease and new code is covered
  • Controllers diagrams are updated when PR changes controllers code (if applicable)

@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Patch coverage: 61.61% and project coverage change: -1.59% ⚠️

Comparison is base (fd8631a) 69.54% compared to head (3119295) 67.95%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #296      +/-   ##
==========================================
- Coverage   69.54%   67.95%   -1.59%     
==========================================
  Files          38       40       +2     
  Lines        3658     4029     +371     
==========================================
+ Hits         2544     2738     +194     
- Misses        865     1006     +141     
- Partials      249      285      +36     
Files Changed Coverage Δ
controllers/controllers.go 0.00% <ø> (ø)
controllers/statusreport/statusreport_adapter.go 45.45% <45.45%> (ø)
status/reporters.go 59.19% <55.50%> (-3.19%) ⬇️
gitops/snapshot_predicate.go 57.14% <57.14%> (ø)
git/github/github.go 82.94% <69.84%> (-4.30%) ⬇️
...ontrollers/statusreport/statusreport_controller.go 87.17% <87.17%> (ø)
gitops/snapshot.go 80.07% <100.00%> (+0.96%) ⬆️
status/status.go 100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

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

@hongweiliu17 hongweiliu17 force-pushed the 524 branch 2 times, most recently from ec97a2e to 767d218 Compare September 4, 2023 11:14
status/reporters.go Outdated Show resolved Hide resolved
@hongweiliu17 hongweiliu17 force-pushed the 524 branch 2 times, most recently from 94f2140 to 265bf56 Compare September 5, 2023 08:07
@hongweiliu17 hongweiliu17 marked this pull request as ready for review September 5, 2023 08:19
gitops/snapshot_predicate.go Outdated Show resolved Hide resolved
@hongweiliu17 hongweiliu17 force-pushed the 524 branch 2 times, most recently from e8f809d to 7cb755a Compare September 5, 2023 09:33
@hongweiliu17 hongweiliu17 changed the base branch from main to statusreport September 20, 2023 02:43
@hongweiliu17
Copy link
Contributor Author

Needs to be targeted to feature branch and commits should be squashed into one. Otherwise LGTM

A feature branch statusreport is created and the pr is updated to point to the feature branch.

@MartinBasti
Copy link
Collaborator

/retest

status/reporters.go Outdated Show resolved Hide resolved
git/github/github_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jsztuka jsztuka left a comment

Choose a reason for hiding this comment

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

lgtm

@hongweiliu17 hongweiliu17 force-pushed the 524 branch 3 times, most recently from 05cd934 to 04a0fb2 Compare September 22, 2023 08:37
* Create function to get credential from snapshot
* Create function to report for result of handle ITSes for snapshot
* Add diagram for statusreport controller

Signed-off-by: Hongwei Liu <[email protected]>
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, nice work!

@hongweiliu17
Copy link
Contributor Author

Thanks all for your review!

@hongweiliu17 hongweiliu17 merged commit a7003a3 into konflux-ci:statusreport Sep 22, 2023
6 checks passed
@hongweiliu17 hongweiliu17 deleted the 524 branch October 11, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants