add revision describe (#15) #16

nimakaviani · 2019-01-30T01:30:34Z

print the yaml formatted revision by default
allow for custom formatting to be passed in

googlebot · 2019-01-30T01:30:37Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

nimakaviani · 2019-01-30T04:56:39Z

I signed it!

googlebot · 2019-01-30T04:56:41Z

CLAs look good, thanks!

cppforlife · 2019-01-30T18:05:22Z

pkg/kn/commands/revision_describe.go

+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"


we should move this line into root command.

fixed in a separate commit in the PR

cppforlife · 2019-01-30T18:06:10Z

pkg/kn/commands/revision_describe.go

+var revisionDescribePrintFlags *genericclioptions.PrintFlags
+
+func NewRevisionDescribeCommand(p *KnParams) *cobra.Command {
+	revisionDescribePrintFlags = genericclioptions.NewPrintFlags("").WithDefaultOutput("yaml")


should use := and remove outside declaration. wanna keep scope as tight as possible.

cppforlife · 2019-01-30T18:07:04Z

pkg/kn/commands/revision_describe_test.go

+
+	action, data, err := fakeRevision([]string{"revision", "describe", "test-rev"}, expectedRevision)
+	if err != nil {
+		t.Error(err)


t.Fatal(err) and below as well.

cppforlife · 2019-01-30T18:07:50Z

pkg/kn/commands/revision_describe_test.go

+		return
+	}
+
+	var returnedRevision = &v1alpha1.Revision{}


more common is to have var returnedRevision *v1alpha1.Revision{} as declaration

evankanderson · 2019-01-30T22:53:37Z

/ok-to-test

…

On Wed, Jan 30, 2019 at 11:29 AM Nima Kaviani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkg/kn/commands/revision_describe.go <#16 (comment)>: > +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "errors" + + "github.com/spf13/cobra" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/cli-runtime/pkg/genericclioptions" + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" fixed in a separate commit in the PR — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#16 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlyN1okTgQ1CFnq0ju1V0MnZWa6xi3fks5vIfKCgaJpZM4aZRa_> .

-- Evan Anderson <argent@google.com>

vdemeester · 2019-01-31T07:45:09Z

pkg/kn/commands/revision_describe.go

@@ -0,0 +1,65 @@
+// Copyright © 2018 The Knative Authors


nit: 2019 🙃

good catch. gotta fix it all across the board though. probably in a separate PR

evankanderson · 2019-01-31T18:25:50Z

I think this only needs to be done on newly-created files, not for existing files, where the copyright starts when the file was created.

…

On Thu, Jan 31, 2019 at 10:09 AM Nima Kaviani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkg/kn/commands/revision_describe.go <#16 (comment)>: > @@ -0,0 +1,65 @@ +// Copyright © 2018 The Knative Authors good catch. gotta fix it all across the board though. probably in a separate PR — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#16 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlyN4s2bKWk42HyNaivLnVS-zrvo_Zuks5vIzFmgaJpZM4aZRa_> .

-- Evan Anderson <argent@google.com>

sixolet · 2019-01-31T22:01:41Z

This looks good to me as soon as the copyright date is changed :)

nimakaviani · 2019-02-01T05:48:02Z

done!

* print the yaml formatted revision by default * allow for custom formatting to be passed in

vdemeester

/lgtm

cppforlife · 2019-02-01T19:27:27Z

/lgtm
/approve

knative-prow-robot · 2019-02-01T19:27:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cppforlife, nimakaviani, vdemeester

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:

~~OWNERS~~ [cppforlife]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* Multiarch github action * Remove upstream workflows from create-release-branch.sh * Patch for ko base image * Remove upstream github actions in update-to-head.sh

knative-prow-robot requested review from evankanderson and mattmoor January 30, 2019 01:30

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 30, 2019

nimakaviani force-pushed the master branch 2 times, most recently from f72d1f2 to fa460dd Compare January 30, 2019 03:36

cppforlife reviewed Jan 30, 2019

View reviewed changes

nimakaviani force-pushed the master branch from fa460dd to 996fd0b Compare January 30, 2019 19:27

put the auth libraries in the root command

11b8912

nimakaviani force-pushed the master branch 2 times, most recently from ca4168c to 2e9b134 Compare January 30, 2019 21:50

knative-prow-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 30, 2019

vdemeester reviewed Jan 31, 2019

View reviewed changes

nimakaviani force-pushed the master branch from 2e9b134 to 7b44fe2 Compare February 1, 2019 05:46

nimakaviani mentioned this pull request Feb 1, 2019

service describe #14

Closed

add revision describe (knative#15)

c7bf3d7

* print the yaml formatted revision by default * allow for custom formatting to be passed in

nimakaviani force-pushed the master branch from 7b44fe2 to c7bf3d7 Compare February 1, 2019 05:59

vdemeester approved these changes Feb 1, 2019

View reviewed changes

knative-prow-robot assigned vdemeester Feb 1, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2019

knative-prow-robot assigned cppforlife Feb 1, 2019

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2019

sixolet merged commit ad636ad into knative:master Feb 1, 2019

cppforlife mentioned this pull request Feb 4, 2019

Add service describe #18

Merged

coryrc pushed a commit to coryrc/client that referenced this pull request May 14, 2020

Also install go dep in prow-tests image (knative#16)

7be111c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add revision describe (#15) #16

add revision describe (#15) #16

nimakaviani commented Jan 30, 2019

googlebot commented Jan 30, 2019

nimakaviani commented Jan 30, 2019

googlebot commented Jan 30, 2019

cppforlife Jan 30, 2019

nimakaviani Jan 30, 2019

cppforlife Jan 30, 2019

cppforlife Jan 30, 2019

cppforlife Jan 30, 2019

evankanderson commented Jan 30, 2019 via email

vdemeester Jan 31, 2019

nimakaviani Jan 31, 2019

evankanderson commented Jan 31, 2019 via email

sixolet commented Jan 31, 2019

nimakaviani commented Feb 1, 2019

vdemeester left a comment

cppforlife commented Feb 1, 2019

knative-prow-robot commented Feb 1, 2019

add revision describe (#15) #16

add revision describe (#15) #16

Conversation

nimakaviani commented Jan 30, 2019

googlebot commented Jan 30, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

nimakaviani commented Jan 30, 2019

googlebot commented Jan 30, 2019

cppforlife Jan 30, 2019

Choose a reason for hiding this comment

nimakaviani Jan 30, 2019

Choose a reason for hiding this comment

cppforlife Jan 30, 2019

Choose a reason for hiding this comment

cppforlife Jan 30, 2019

Choose a reason for hiding this comment

cppforlife Jan 30, 2019

Choose a reason for hiding this comment

evankanderson commented Jan 30, 2019 via email

vdemeester Jan 31, 2019

Choose a reason for hiding this comment

nimakaviani Jan 31, 2019

Choose a reason for hiding this comment

evankanderson commented Jan 31, 2019 via email

sixolet commented Jan 31, 2019

nimakaviani commented Feb 1, 2019

vdemeester left a comment

Choose a reason for hiding this comment

cppforlife commented Feb 1, 2019

knative-prow-robot commented Feb 1, 2019