-
Notifications
You must be signed in to change notification settings - Fork 289
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
Curated packages bugfixes #2045
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## main #2045 +/- ##
==========================================
- Coverage 49.81% 49.73% -0.08%
==========================================
Files 305 306 +1
Lines 25717 25791 +74
==========================================
+ Hits 12811 12828 +17
- Misses 11583 11640 +57
Partials 1323 1323
Continue to review full report at Codecov.
|
@@ -56,9 +57,9 @@ func getResources(ctx context.Context, resourceType string, output string, args | |||
return fmt.Errorf("kubectl execution failure: \n%v", err) | |||
} | |||
if len(stdOut.Bytes()) == 0 { | |||
fmt.Printf("No resources found in %v namespace", constants.EksaPackagesName) | |||
fmt.Printf("No resources found in %v namespace\n", constants.EksaPackagesName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this make use of logger instead of a printf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this needs to be printed to the user.
@@ -68,6 +68,7 @@ func (c *Create) Run(ctx context.Context, clusterSpec *cluster.Spec, validator i | |||
} | |||
|
|||
if packagesLocation != "" { | |||
curatedpackages.PrintLicense() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of printing out the license all the time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a business requirement to print out the license.
/lgtm |
* Fix some minor issues * Add License during cluster creation * Add License in controller and package installation * Add formatting curated packages license * Gofumpt resolution * Fix PR Review Comments * Fix PR Review Comments
Description of changes:
Add License for Curated Packages
Provides the capability to print a license during the following scenarios:
Minor Bugfixes
Move Dependency creation to curatedpackages
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.