search

kubevirt / kubevirt.github.io

KubeVirt website repo, documentation at https://kubevirt.io/user-guide/
https://kubevirt.io
MIT License
30 stars 112 forks source link

Update README with new check_lint targets. #777

Closed tylerauerbeck closed 3 years ago

tylerauerbeck commented 3 years ago

Signed-off-by: Tyler Auerbeck tauerbec@redhat.com

What this PR does / why we need it: This updates the README with new information around the check_lint make target.

Does this PR fix any issue? _(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged)_:

N/A

Special notes for your reviewer:

kubevirt-bot commented 3 years ago

Hi @tylerauerbeck. Thanks for your PR.

I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cwilkers commented 3 years ago

/ok-to-test

mazzystr commented 3 years ago

@tylerauerbeck I notice the make help target doesn't actually display the check_lint target. Can you update Makefile and include a patch to this pull req? Thanks!

tylerauerbeck commented 3 years ago

@mazzystr @cwilkers Fixed. Not sure why the dco signoff is missing as I see the sign off in the commit. Let me know if there's something I need to do to get this corrected.

mazzystr commented 3 years ago

@tylerauerbeck Thanks for creating that issue for CONTRIBUTING. I didn't realize it was missing. Our governance docs are kind of a mess.

For right now please amend your commit. git commits must have a signature. Directions are in the README

cwilkers commented 3 years ago

@tylerauerbeck It looks like the second commit isn't signed according to the DCO checker. Are you sure you didn't just add the signed-off-by text? There has to be a proper cryptographic signature put there by git. (I'm just guessing, it would be a research spike for me to figure out how to properly check this.)

tylerauerbeck commented 3 years ago

@cwilkers Yepp. I just did git commit --amend --signoff, but let me just see if I can get this cleaned up.

mazzystr commented 3 years ago

/retest

mazzystr commented 3 years ago

/test all

cwilkers commented 3 years ago

DCO is still unhappy. I wonder if it just missed the cue to rerun the check after the last commit. Perhaps if you force push either a simple comment update (like add a space or something somewhere) or if you squash the two commits and make sure that update is signed.

mazzystr commented 3 years ago

/approve

mazzystr commented 3 years ago

@tylerauerbeck I have no idea why DCO won't recheck that second commit.

Can you git rebase -i HEAD~2 and combine in to one commit? Maybe that will get things moving. Make sure your commit message contains Signed-off-by: you

tylerauerbeck commented 3 years ago

@mazzystr Yepp. Went ahead and rebased and things look good.

mazzystr commented 3 years ago

Thanks for that rebase! Now we roll!

/lgtm /approve

On Thu, Aug 26, 2021, 5:15 PM Tyler Auerbeck @.***> wrote:

@mazzystr https://github.com/mazzystr Yepp. Went ahead and rebased and things look good.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubevirt/kubevirt.github.io/pull/777#issuecomment-906827163, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS5KZ4WD4JPSPGGF4MKYXTT63KK3ANCNFSM5CZZV2UA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

kubevirt-bot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mazzystr

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](https://github.com/kubevirt/kubevirt.github.io/blob/master/OWNERS)~~ [mazzystr] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment