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

initial code for integration context #24

Merged
merged 1 commit into from
Sep 11, 2018

Conversation

lburgazzoli
Copy link
Contributor

No description provided.

@lburgazzoli lburgazzoli force-pushed the integration-context branch 2 times, most recently from c3f3f54 to 7fe398f Compare September 11, 2018 13:01
Copy link
Member

@nicolaferraro nicolaferraro left a comment

Choose a reason for hiding this comment

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

Looks really good. Just few minor issues.

Remember to add docs when you add the final version 😄 !

Gopkg.lock Outdated
@@ -2,230 +2,179 @@


[[projects]]
digest = "1:5c3894b2aa4d6bead0ceeea6831b305d62879c871780e7b76296ded1b004bc57"
Copy link
Member

Choose a reason for hiding this comment

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

Can you move to "dep 0.5.0"?

@@ -36,6 +36,7 @@ type Integration struct {
metav1.ObjectMeta `json:"metadata"`
Spec IntegrationSpec `json:"spec"`
Status IntegrationStatus `json:"status,omitempty"`
Context string `json:"context,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This should be part of Spec according to Kubernetes conventions


const (
// IntegrationContextPhaseEditing --
IntegrationContextPhaseEditing IntegrationContextPhase = "Editing"
Copy link
Member

Choose a reason for hiding this comment

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

Is this a real status? Like "Draft"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, renamed it to draft to make it more clear

}

cmd.Flags().BoolVarP(&impl.discard, "discard", "x", false, "Discard current edit session")
cmd.Flags().BoolVarP(&impl.save, "save", "s", false, "Save edit session")
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't seem you're opening a session (I hope you're not), it's a patch like action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavior changed so the context is saved by default, if you want to edit the context using multiple commands, set --save=false

}

func (action *integrationContexMonitorAction) Name() string {
return "Edit"
Copy link
Member

Choose a reason for hiding this comment

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

Ok they're named the same but it's everything todo

Copy link
Member

@nicolaferraro nicolaferraro left a comment

Choose a reason for hiding this comment

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

Perfect!

@nicolaferraro nicolaferraro merged commit ab51a14 into apache:master Sep 11, 2018
@lburgazzoli lburgazzoli deleted the integration-context branch September 11, 2018 13:59
zach-robinson pushed a commit to zach-robinson/camel-k that referenced this pull request May 28, 2021
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.

None yet

3 participants