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(RHTAPREL-253): queue Releases #366

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidmogar
Copy link
Collaborator

No description provided.

@davidmogar davidmogar requested a review from a team as a code owner February 19, 2024 16:41
@davidmogar davidmogar marked this pull request as draft February 19, 2024 16:41
@davidmogar davidmogar force-pushed the rhtaprel253 branch 2 times, most recently from 999a37d to 90da749 Compare February 20, 2024 09:00
Copy link

sonarcloud bot commented Feb 20, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue

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

See analysis details on SonarCloud

Copy link
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

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

I like the approach. I reviewed most but not all of the code

loader/loader_test.go Outdated Show resolved Hide resolved
api/v1alpha1/release_types.go Show resolved Hide resolved
api/v1alpha1/release_types.go Show resolved Hide resolved
Signed-off-by: David Moreno García <[email protected]>
Copy link

sonarcloud bot commented Apr 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

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

Project coverage is 83.55%. Comparing base (d505670) to head (e3af994).
Report is 1 commits behind head on main.

Files Patch % Lines
loader/loader.go 0.00% 17 Missing ⚠️
controllers/release/adapter.go 60.00% 2 Missing and 2 partials ⚠️
loader/loader_mock.go 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
- Coverage   84.60%   83.55%   -1.06%     
==========================================
  Files          26       26              
  Lines        1384     1423      +39     
==========================================
+ Hits         1171     1189      +18     
- Misses        153      172      +19     
- Partials       60       62       +2     

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

@@ -44,6 +45,35 @@ func NewLoader() ObjectLoader {
return &loader{}
}

// GetActiveManagedReleasePipelineRuns returns all active managed Release PipelineRuns for the Application being Released.
// PipelineRuns for the Release passed as an argument are ignored.
func (l *loader) GetActiveManagedReleasePipelineRuns(ctx context.Context, cli client.Client, release *v1alpha1.Release) (*tektonv1.PipelineRunList, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use unfinished or something instead of active? We already have active for the RP/RPA stuff

}
return controller.RequeueAfter(time.Minute, nil)
}

pipelineRun, err := a.loader.GetManagedReleasePipelineRun(a.ctx, a.client, a.release)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't pipelineRuns.length be 1 if there is already a pipelineRun running for this release? So we don't have to fetch it again?

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.

4 participants