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

parameterize VersionsInSummary #3683

Closed
wants to merge 3 commits into from
Closed

parameterize VersionsInSummary #3683

wants to merge 3 commits into from

Conversation

satya-dillikar
Copy link
Contributor

Signed-off-by: “satya-dillikar” <“satya.dillikar@gmail.com”>

Description of the change

Parameterize the func packageAppVersionsSummary to take a new param.

Benefits

In future, This new parameter VersionsInSummary can be populated from the configuration files.

Possible drawbacks

None

Applicable issues

Additional information

More proper fix is still pending.

Signed-off-by: “satya-dillikar” <“satya.dillikar@gmail.com”>
Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Thanks Satya. I'll wait for the tests for packageAppVersionsSummary to be updated (usually a good idea to start there, adding the new behaviour to get a failing test, then updating the code to get a passing test etc.).

}

// packageAppVersionsSummary converts the model chart versions into the required version summary.
func packageAppVersionsSummaryImpl(versions []models.ChartVersion, versionInSummary VersionsInSummary) []*corev1.PackageAppVersion {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any reason to create a new function? Why not just update the signature of the existing packageAppVersionsSummary function and update the (only) call-site to just pass in the extra VersionsInSummary{MajorVersionsInSummary, MinorVersionsInSummary, PatchVersionsInSummary} ?

“satya-dillikar” added 2 commits October 28, 2021 17:39
Signed-off-by: “satya-dillikar” <“satya.dillikar@gmail.com”>
Signed-off-by: “satya-dillikar” <“satya.dillikar@gmail.com”>
@satya-dillikar
Copy link
Contributor Author

Create a new PR: #3703
Dropping this PR due to more unwanted files in these checkins.

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

Successfully merging this pull request may close these issues.

Some Chart versions hidden from dropdown when deploying an application
2 participants