-
Notifications
You must be signed in to change notification settings - Fork 243
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
Migrate devfile cmd validation to validate pkg #3912
Migrate devfile cmd validation to validate pkg #3912
Conversation
Skipping CI for Draft Pull Request. |
Codecov Report
@@ Coverage Diff @@
## master #3912 +/- ##
==========================================
- Coverage 44.55% 44.53% -0.03%
==========================================
Files 143 145 +2
Lines 13979 14005 +26
==========================================
+ Hits 6229 6237 +8
- Misses 7158 7176 +18
Partials 592 592
Continue to review full report at Codecov.
|
e3250ad
to
1ec61ca
Compare
// func TestIsContainer(t *testing.T) { | ||
|
||
// tests := []struct { | ||
// name string | ||
// component common.DevfileComponent | ||
// wantIsSupported bool | ||
// }{ | ||
// { | ||
// name: "Case 1: Container component", | ||
// component: testingutil.GetFakeContainerComponent("comp1"), | ||
// wantIsSupported: true, | ||
// }, | ||
// { |
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.
these comments need to be removed
|
||
// Successful | ||
return d, nil | ||
return d, err |
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.
I think these changes can be applied in the above functions as well
pkg/devfile/validate/commands.go
Outdated
// validateCommands validates all the devfile commands | ||
func validateCommands(commands map[string]common.DevfileCommand, components []common.DevfileComponent) (err error) { | ||
|
||
// commandsMap := adapterCommon.GetCommandsMap(commands) |
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.
I think this can be removed
pkg/devfile/validate/commands.go
Outdated
// If the command is a composite command, need to validate that it is valid | ||
if command.Composite != nil { | ||
parentCommands := make(map[string]string) | ||
// commandsMap := adapterCommon.GetCommandsMap(commands) |
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.
I think this comment can be removed
Tests seem to be failing on Travis |
1ec61ca
to
4e6bee1
Compare
/retest |
4e6bee1
to
f62df5c
Compare
CI seems to be failing
|
@mik-dass yep, I noticed this yesterday |
f62df5c
to
6a5c191
Compare
/retest |
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.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: girishramnani The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
6a5c191
to
7bc997d
Compare
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.
Readding the lgtm label due to rebase
/lgtm
What type of PR is this?
/kind cleanup
/kind code-refactoring
What does does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #3703
PR acceptance criteria:
Unit test
Integration test
Documentation
I have read the test guidelines
How to test changes / Special notes to the reviewer: