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

Make pkg/install/Deployment podTemplateOptions bool functions accept bool param #7942

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

kaovilai
Copy link
Contributor

Signed-off-by: Tiger Kaovilai [email protected]

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #7379

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@kaovilai
Copy link
Contributor Author

/kind changelog-not-required

@github-actions github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jun 27, 2024
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 26.66667% with 11 lines in your changes missing coverage. Please review.

Project coverage is 58.82%. Comparing base (c827fd0) to head (bd2008c).
Report is 28 commits behind head on main.

Files Patch % Lines
pkg/install/deployment.go 40.00% 6 Missing ⚠️
pkg/install/resources.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7942      +/-   ##
==========================================
+ Coverage   58.79%   58.82%   +0.02%     
==========================================
  Files         345      346       +1     
  Lines       28766    28901     +135     
==========================================
+ Hits        16914    17002      +88     
- Misses      10423    10467      +44     
- Partials     1429     1432       +3     

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

@blackpiglet
Copy link
Contributor

blackpiglet commented Jun 27, 2024

IMO, this doesn't make a difference from the original functions.
We can set the --restore-only as false by not calling the WithRestoreOnly() function.

@kaovilai
Copy link
Contributor Author

kaovilai commented Jun 27, 2024

@blackpiglet the alternative would be to make podTemplateOption into PodTemplateOption so that i can form dynamically an array of []PodTemplateOption functions outside of this package.

@kaovilai
Copy link
Contributor Author

for context this is for external consumption of these functions

See: #7379 (comment)

WIthout this Bool func that accepts a param, or alternatively exporting the type, I cannot store these podTemplateOption functions in a slice to pass to install.Deployment(slice...) outside of the install package.

@blackpiglet
Copy link
Contributor

for context this is for external consumption of these functions

See: #7379 (comment)

WIthout this Bool func that accepts a param, or alternatively exporting the type, I cannot store these podTemplateOption functions in a slice to pass to install.Deployment(slice...) outside of the install package.

I see. I suggest refactoring those existing functions to suit your scenario instead of keeping the old and the new ones together.

@kaovilai kaovilai marked this pull request as draft July 1, 2024 12:46
@kaovilai kaovilai marked this pull request as ready for review July 12, 2024 03:52
@github-actions github-actions bot requested a review from reasonerjt July 12, 2024 03:52
@reasonerjt reasonerjt merged commit 5b9b8e7 into vmware-tanzu:main Jul 25, 2024
68 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-unit-tests kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make pkg/install/Deployment podTemplateOptions bool functions accept bool param
3 participants