Add separate "projects" cmd to list all projects #9199

juanvallejo · 2016-06-07T14:31:49Z

liggitt · 2016-06-07T14:33:40Z

pkg/cmd/cli/cmd/project.go

@@ -66,7 +66,6 @@ func NewCmdProject(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.
 		Short:   "Switch to another project",
 		Long:    projectLong,
 		Example: fmt.Sprintf(projectExample, fullName),
-		Aliases: []string{"projects"},


will this break existing uses?

So far, the only test against an oc projects command was to make sure it returned the same --help output as oc project, I have updated this, but aside from running new and existing tests on it, I haven't found any conflicts

alias. ouch.. yes it will

I mean uses by the world at large, which we can't refactor :)

I mean uses by the world at large, which we can't refactor :)

I suspect that @fabianofranz may take it anyway if he thinks its a usability gain. Realistically the exposure is small, non-scripts may expect this behavior anyway and oc project was never something reason to script against.

I'm ok with this being a breaking change given the gains. But I'd like to minimize the breakage by making the exact current message still present in the output message of both project and projects.

$ oc project Using project "awesome" on server "https://127.0.0.1:8443". $ oc projects You have access to the following projects and can switch between them with 'oc project <projectname>': * awesome (current) * notsoawesome * okone * weirdo Using project "awesome" on server "https://127.0.0.1:8443".

That's also consistent with how we print the projects after logging in, and printing the current one at the end should address the use cases of having a long list of projects.

Comments?

@Kargakis we consider ok to use aliases for things like plural/singular. But aliases should not be "smart" so for for anything other than that (e.g. remove/delete) we must use suggestions like in $ oc remove\n did you mean delete?.

juanvallejo · 2016-06-07T14:34:03Z

While logged in as a test user:
oc login -u test -p test
oc projects

You are not a member of any projects. You can request a project to be created with the 'new-project' command.

oc new-project test
oc new-project test2

oc projects

Using project "test2" on server "https://10.0.2.15:8443".
Your projects are:
- test
* test2

0xmichalis · 2016-06-07T14:48:49Z

Why do we need a separate command for this? Isn't oc get projects enough?

juanvallejo · 2016-06-07T15:01:14Z

@Kargakis didn't know about oc get projects. oc projects does display the current project (and its server) as well. It could be a hybrid between oc project and oc get projects

0xmichalis · 2016-06-07T15:03:22Z

@juanvallejo you should be able to add that info in the resource printer for projects and display anything additional in oc get projects.

0xmichalis · 2016-06-07T15:06:00Z

Also tbh I don't mind breaking people who run oc projects. Upstream kubectl policy is not to use aliases and I think I agree with it. But I don't want a new command for it.

fabianofranz · 2016-06-07T15:59:24Z

[test]

juanvallejo · 2016-06-07T19:13:40Z

@fabianofranz or @deads2k Not sure if I can ask rosie-bot to re-test

fabianofranz · 2016-06-07T21:54:23Z

re[test]

fabianofranz · 2016-06-08T16:15:06Z

pkg/cmd/cli/cmd/projects.go

+
+	return nil
+}
+func (o ProjectsOptions) Validate() error {


This can be removed.

fabianofranz · 2016-06-08T16:37:50Z

flake, [test] again

juanvallejo · 2016-06-08T20:46:06Z

[test]

juanvallejo · 2016-06-09T15:30:48Z

re[test]

fabianofranz · 2016-06-10T17:50:00Z

pkg/cmd/cli/cmd/projects.go

+	"fmt"
+	"io"
+
+	// kapi "k8s.io/kubernetes/pkg/api"


delete if not needed.

fabianofranz · 2016-06-10T18:57:57Z

@juanvallejo Minor comments, please address then LGTM.

0xmichalis · 2016-06-11T11:43:52Z

pkg/cmd/cli/cmd/projects.go

+type ProjectsOptions struct {
+	Config       clientcmdapi.Config
+	ClientConfig *restclient.Config
+	ClientFn     func() (*client.Client, error)


Use a ProjectNamespacer instead of a callback

fabianofranz · 2016-06-14T14:23:03Z

pkg/cmd/cli/cmd/projects.go

+		case 0:
+			msg += "You are not a member of any projects. You can request a project to be created with the 'new-project' command."
+		case 1:
+			msg += fmt.Sprintf("\nYou have one project on this server: %s", api.DisplayNameAndNameForProject(&projects[0]))


I think \n is not needed here, it would add a linebreak where other cases don't have it. Also, use %q instead of %s so that you have double quotes in the project name, and add a "." after it at the end of the phrase.

fabianofranz · 2016-06-14T14:23:58Z

@juanvallejo added one minor comment, let me know when it's done. ;)

juanvallejo · 2016-06-14T14:44:53Z

@fabianofranz Thanks for the feedback, I have updated the output according to your comment

openshift-bot · 2016-06-14T15:01:41Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4873/) (Image: devenv-rhel7_4377)

juanvallejo · 2016-06-14T16:18:05Z

re[test]

openshift-bot · 2016-06-14T16:21:07Z

Evaluated for origin test up to 9603c66

openshift-bot · 2016-06-14T17:30:22Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4840/)

juanvallejo · 2016-06-14T17:32:09Z

[merge]

fabianofranz · 2016-06-15T02:16:56Z

[merge]

openshift-bot · 2016-06-15T02:20:40Z

Evaluated for origin merge up to 9603c66

deads2k · 2016-06-15T12:14:46Z

[merge]

@juanvallejo nice try :)

0xmichalis · 2016-06-20T14:43:01Z

@fabianofranz can we highlight the current project either by a different color or by using only one star instead of using (current)?

juanvallejo · 2016-06-20T17:37:19Z

@Kargakis

can we highlight the current project either by a different color or by using only one star instead of using (current)

I like this suggestion, see #9439, it displays the current project (in a list of two or more) with bold green font and an asterisk in front.

juanvallejo assigned deads2k, fabianofranz and 0xmichalis Jun 7, 2016

liggitt reviewed Jun 7, 2016
View reviewed changes

fabianofranz reviewed Jun 8, 2016
View reviewed changes

pkg/cmd/cli/cmd/projects.go

return nil

}

func (o ProjectsOptions) Validate() error {

Copy link

Member

fabianofranz Jun 8, 2016

Choose a reason for hiding this comment

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

This can be removed.

juanvallejo force-pushed the jvallejo_add-projects-cmd branch from 0635d3a to c8080d0 Compare June 9, 2016 03:59

fabianofranz reviewed Jun 10, 2016
View reviewed changes

pkg/cmd/cli/cmd/projects.go

"fmt"

"io"

// kapi "k8s.io/kubernetes/pkg/api"

Copy link

Member

fabianofranz Jun 10, 2016

Choose a reason for hiding this comment

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

delete if not needed.

0xmichalis reviewed Jun 11, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_add-projects-cmd branch from d8485e4 to 8f9bfd9 Compare June 13, 2016 21:34

fabianofranz reviewed Jun 14, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_add-projects-cmd branch from 8f9bfd9 to 4367497 Compare June 14, 2016 14:41

update "oc projects" to display all projects

9603c66

juanvallejo force-pushed the jvallejo_add-projects-cmd branch from 4367497 to 9603c66 Compare June 14, 2016 16:17

openshift-bot merged commit 6ab98bc into openshift:master Jun 15, 2016

juanvallejo deleted the jvallejo_add-projects-cmd branch June 15, 2016 14:41

juanvallejo mentioned this pull request Jun 20, 2016

Update oc projects to mark current project with asterisk #9439

Merged

Add separate "projects" cmd to list all projects #9199

Add separate "projects" cmd to list all projects #9199

Conversation

juanvallejo commented Jun 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanvallejo commented Jun 7, 2016 • edited Loading

0xmichalis commented Jun 7, 2016

juanvallejo commented Jun 7, 2016 • edited Loading

0xmichalis commented Jun 7, 2016

0xmichalis commented Jun 7, 2016 • edited Loading

fabianofranz commented Jun 7, 2016

juanvallejo commented Jun 7, 2016 • edited Loading

fabianofranz commented Jun 7, 2016

Choose a reason for hiding this comment

fabianofranz commented Jun 8, 2016

juanvallejo commented Jun 8, 2016

juanvallejo commented Jun 9, 2016

Choose a reason for hiding this comment

fabianofranz commented Jun 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabianofranz commented Jun 14, 2016

juanvallejo commented Jun 14, 2016

openshift-bot commented Jun 14, 2016 • edited Loading

juanvallejo commented Jun 14, 2016

openshift-bot commented Jun 14, 2016

openshift-bot commented Jun 14, 2016

juanvallejo commented Jun 14, 2016

fabianofranz commented Jun 15, 2016

openshift-bot commented Jun 15, 2016

deads2k commented Jun 15, 2016

0xmichalis commented Jun 20, 2016

juanvallejo commented Jun 20, 2016

juanvallejo commented Jun 7, 2016 •

edited

Loading

juanvallejo commented Jun 7, 2016 •

edited

Loading

0xmichalis commented Jun 7, 2016 •

edited

Loading

juanvallejo commented Jun 7, 2016 •

edited

Loading

openshift-bot commented Jun 14, 2016 •

edited

Loading