Skip to content

Commit

Permalink
Address review comments (#3267)
Browse files Browse the repository at this point in the history
  • Loading branch information
Aditi Sharma committed Jun 2, 2020
1 parent e558170 commit bf8fed0
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 22 deletions.
15 changes: 9 additions & 6 deletions pkg/devfile/adapters/common/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
// GetCommand iterates through the devfile commands and returns the associated devfile command
func getCommand(data data.DevfileData, commandName string, groupType common.DevfileCommandGroupType) (supportedCommand common.DevfileCommand, err error) {

for _, command := range data.GetCommands() {
commands := data.GetCommands()

for _, command := range commands {

command = updateGroupforCustomCommand(commandName, groupType, command)

Expand Down Expand Up @@ -43,7 +45,7 @@ func getCommand(data data.DevfileData, commandName string, groupType common.Devf
// group:
// kind: build
if command.Exec.Group.Kind != groupType {
return supportedCommand, fmt.Errorf("mismatched type, command %s is of type %v groupType in devfile", commandName, groupType)
return supportedCommand, fmt.Errorf("mismatched group kind, command %s is of group kind %v groupType in devfile", commandName, groupType)

}
supportedCommand = command
Expand All @@ -62,7 +64,7 @@ func getCommand(data data.DevfileData, commandName string, groupType common.Devf

if commandName == "" {
// if default command is not found return the first command found for the matching type.
for _, command := range data.GetCommands() {
for _, command := range commands {
if command.Exec.Group.Kind == groupType {
supportedCommand = command
return supportedCommand, nil
Expand Down Expand Up @@ -92,6 +94,7 @@ func getCommand(data data.DevfileData, commandName string, groupType common.Devf
// 1. command has to be of type exec
// 2. component should be present
// 3. command should be present
// 4. command must have group
func validateCommand(data data.DevfileData, command common.DevfileCommand) (err error) {

// type must be exec
Expand All @@ -116,13 +119,13 @@ func validateCommand(data data.DevfileData, command common.DevfileCommand) (err
// must map to a supported component
components := GetSupportedComponents(data)

isActionValid := false
isComponentValid := false
for _, component := range components {
if command.Exec.Component == component.Container.Name {
isActionValid = true
isComponentValid = true
}
}
if !isActionValid {
if !isComponentValid {
return fmt.Errorf("the command does not map to a supported component")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/devfile/adapters/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type ComponentInfo struct {
ContainerName string
}

// PushCommandsMap stores the commands to be executed as per there types.
// PushCommandsMap stores the commands to be executed as per their types.
type PushCommandsMap map[common.DevfileCommandGroupType]common.DevfileCommand

// NewPushCommandMap returns the instance of PushCommandsMap
Expand Down
3 changes: 1 addition & 2 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,7 @@ func (a Adapter) execDevfile(commandsMap common.PushCommandsMap, componentExists
PodName: podName,
}

// Only add runinit to the expected commands if the component doesn't already exist
// This would be the case when first running the container
// only execute Init command, if it is first run of container.
if !componentExists {
// Get Init Command
command, ok := commandsMap[versionsCommon.InitCommandGroupType]
Expand Down
29 changes: 16 additions & 13 deletions pkg/devfile/parser/data/common/types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package common

// ProjectSourceType describes the type of Project sources.
// DevfileProjectSourceType describes the type of Project sources.
// Only one of the following project sources may be specified.
type DevfileProjectSourceType string

Expand All @@ -11,6 +11,8 @@ const (
CustomProjectSourceType DevfileProjectSourceType = "Custom"
)

// DevfileComponentType describes the type of component.
// Only one of the following component type may be specified
type DevfileComponentType string

const (
Expand All @@ -22,6 +24,8 @@ const (
CustomComponentType DevfileComponentType = "Custom"
)

// DevfileCommandType describes the type of command.
// Only one of the following command type may be specified.
type DevfileCommandType string

const (
Expand All @@ -32,8 +36,7 @@ const (
CustomCommandType DevfileCommandType = "Custom"
)

// CommandGroupType describes the kind of command group.
// +kubebuilder:validation:Enum=build;run;test;debug
// DevfileCommandGroupType describes the kind of command group.
type DevfileCommandGroupType string

const (
Expand All @@ -45,17 +48,17 @@ const (
InitCommandGroupType DevfileCommandGroupType = "init"
)

// Metadata Optional metadata
// DevfileMetadata metadata for devfile
type DevfileMetadata struct {

// Optional devfile name
// Name Optional devfile name
Name string `json:"name,omitempty"`

// Optional semver-compatible version
// Version Optional semver-compatible version
Version string `json:"version,omitempty"`
}

// CommandsItems
// DevfileCommand command specified in devfile
type DevfileCommand struct {

// Exec command
Expand All @@ -65,7 +68,7 @@ type DevfileCommand struct {
Type DevfileCommandType `json:"type,omitempty"`
}

// ComponentsItems
// DevfileComponent component specified in devfile
type DevfileComponent struct {

// CheEditor component
Expand All @@ -86,14 +89,14 @@ type DevfileComponent struct {
// Openshift component
Openshift *Openshift `json:"openshift,omitempty"`

// Type of project source
// Type of component
Type DevfileComponentType `json:"type,omitempty"`

// Volume component
Volume *Volume `json:"volume,omitempty"`
}

// ProjectsItems
// DevfileProject project defined in devfile
type DevfileProject struct {

// Path relative to the root of the projects to which this project should be cloned into. This is a unix-style relative path (i.e. uses forward slashes). The path is invalid if it is absolute or tries to escape the project root through the usage of '..'. If not specified, defaults to the project name.
Expand Down Expand Up @@ -234,7 +237,7 @@ type Env struct {
Value string `json:"value"`
}

// Events Bindings of commands to events. Each command is referred-to by its name.
// DevfileEvents events Bindings of commands to events. Each command is referred-to by its name.
type DevfileEvents struct {

// Names of commands that should be executed after the workspace is completely started. In the case of Che-Theia, these commands should be executed after all plugins and extensions have started, including project cloning. This means that those commands are not triggered until the user opens the IDE in his browser.
Expand Down Expand Up @@ -352,7 +355,7 @@ type Openshift struct {
Url string `json:"url,omitempty"`
}

// Parent Parent workspace template
// DevfileParent Parent workspace template
type DevfileParent struct {

// Reference to a Kubernetes CRD of type DevWorkspaceTemplate
Expand Down Expand Up @@ -384,7 +387,7 @@ type Volume struct {
Size string `json:"size,omitempty"`
}

// VolumeMountsItems Volume that should be mounted to a component container
// VolumeMount Volume that should be mounted to a component container
type VolumeMount struct {

// The volume mount name is the name of an existing `Volume` component. If no corresponding `Volume` component exist it is implicitly added. If several containers mount the same volume name then they will reuse the same volume and will be able to access to the same files.
Expand Down

0 comments on commit bf8fed0

Please sign in to comment.