Skip to content

Conversation

@olivercodes
Copy link
Member

This is to resolve #37

After chatting with @astoycos we decided it would be better to follow what other sigs have done, and implement a prow job, as well as follow similar patterns for the test scripts (moving to live under hack/).

There will be a second PR for this issue, in test-infra to add the prow job that calls these changes.

p.s. This is my first PR to a kubernetes project, so I'm sure I did something wrong. Please feel free to tell me :).

@netlify
Copy link

netlify bot commented Nov 1, 2022

Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name Link
🔨 Latest commit 6e6b1a2
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/6362876eaefe8d00091363ec
😎 Deploy Preview https://deploy-preview-47--kubernetes-sigs-network-policy-api.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 1, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: olivercodes / name: Bryan Oliver (b534b9b)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Nov 1, 2022
@k8s-ci-robot
Copy link
Contributor

Welcome @olivercodes!

It looks like this is your first PR to kubernetes-sigs/network-policy-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/network-policy-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Nov 1, 2022
@olivercodes olivercodes changed the title first pass at scripts for prow precommit and crd jobs scripts for prow precommit and crd jobs Nov 1, 2022
@astoycos astoycos self-assigned this Nov 2, 2022
Copy link
Member

@astoycos astoycos left a comment

Choose a reason for hiding this comment

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

Thanks so much for tackling this!!! @olivercodes

This is almost there.

When I run locally I am running into some errors

[astoycos@localhost network-policy-api]$ make verify 
hack/verify-all.sh -v
Skipping hack/../hack/verify-all.sh
Verifying gofmt
Test FAILED: hack/../hack/verify-gofmt.sh 
level=warning msg="[linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649."
SUCCESS: hack/../hack/verify-golint.sh 
Verifying govet
pattern ./...: lstat /home/astoycos/go/src/github.com/k8s.io/network-policy-api/cache/00/003426ff6eb11828a6df4b2609e8ee569ba24474bc5a96c630c74d47cf5ee675-a: permission denied
no Go files in /home/astoycos/go/src/github.com/k8s.io/network-policy-api
Test FAILED: hack/../hack/verify-govet.sh 
make: *** [Makefile:41: verify] Error 1

@astoycos
Copy link
Member

astoycos commented Nov 2, 2022

/assign @tssurya

As well if she has some extra time for a look

@tssurya
Copy link
Contributor

tssurya commented Nov 2, 2022

woohoo thanks for your contribution @olivercodes!, we were just about to start on the CI parts for the repo and this is great a start, let me try to review this by end of this week. - first thing that came to my mind as I did an overall scroll was if you could squash the cleanup commits that would make history look cleaner..

@olivercodes olivercodes force-pushed the fmt-vet-lint-crd-scripts branch from 95fbc77 to 7a16680 Compare November 2, 2022 12:56
Copy link
Member

@astoycos astoycos left a comment

Choose a reason for hiding this comment

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

@olivercodes This is pretty much there!! Thanks again.

One last nit and also if you could just Sign-off your commit/s while you're at it that'd be awesome!

(I need to work on a contributor guidelines doc or point back to K8s's https://github.com/kubernetes/community/blob/master/contributors/devel/README.md#setting-up-your-dev-environment-coding-and-debugging in our readme)

@olivercodes olivercodes force-pushed the fmt-vet-lint-crd-scripts branch from 7a16680 to 6e6b1a2 Compare November 2, 2022 15:06
@olivercodes
Copy link
Member Author

@olivercodes This is pretty much there!! Thanks again.

One last nit and also if you could just Sign-off your commit/s while you're at it that'd be awesome!

(I need to work on a contributor guidelines doc or point back to K8s's https://github.com/kubernetes/community/blob/master/contributors/devel/README.md#setting-up-your-dev-environment-coding-and-debugging in our readme)

Done, and think I got it :)
image

Fixed that last nit and squased

@astoycos
Copy link
Member

astoycos commented Nov 2, 2022

Awesome!!! Great first Contribution :)

Let's merge everything and see it all working!

@astoycos
Copy link
Member

astoycos commented Nov 2, 2022

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 2, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astoycos, olivercodes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot merged commit 732ceb8 into kubernetes-sigs:master Nov 2, 2022
@olivercodes olivercodes deleted the fmt-vet-lint-crd-scripts branch November 2, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Gofmt and Govet gh actions

4 participants