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

Fix: example manager and instructions #48

Merged
merged 12 commits into from
May 19, 2023

Conversation

semmet95
Copy link
Contributor

@semmet95 semmet95 commented May 10, 2023

Description of your changes

Updated ClusterRole and Quick Start guide.
Fixes #47

Fixes #

  • rules[0].apiGroups: Required value: resource rules must supply at least one api group error.

I have:

  • Read and followed KubeVela's contribution process.
  • Add related tests.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

By running the quick tutorial with the updated examples.

Special notes for your reviewer

I have updated the Quick Start guide's Prerequisites section to including enabling the kube-trigger addon

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.93 ⚠️

Comparison is base (781061d) 75.23% compared to head (4895b15) 73.30%.

❗ Current head 4895b15 differs from pull request most recent head b358fd1. Consider uploading reports for the commit b358fd1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
- Coverage   75.23%   73.30%   -1.93%     
==========================================
  Files          12       15       +3     
  Lines        1078     1195     +117     
==========================================
+ Hits          811      876      +65     
- Misses        229      275      +46     
- Partials       38       44       +6     
Flag Coverage Δ
unittests 73.30% <ø> (-1.93%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@FogDong FogDong changed the title Fix/example manager and instructions Fix: example manager and instructions May 10, 2023
@FogDong
Copy link
Member

FogDong commented May 10, 2023

Please sign the DCO with git commit -s

@@ -5,7 +5,9 @@ metadata:
creationTimestamp: null
name: kube-trigger-manager-role
rules:
- resources:
- apiGroups:
Copy link
Member

Choose a reason for hiding this comment

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

I think this file is auto generated, can you help to check that is there anything wrong with the generator? cc @charlie0129

Copy link
Member

@charlie0129 charlie0129 May 10, 2023

Choose a reason for hiding this comment

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

Yes, that's right. Maybe newer versions of kubectl require apiGroups to be set.

I fixed the generator to generate that field in the latest commit.

Copy link
Member

@charlie0129 charlie0129 left a comment

Choose a reason for hiding this comment

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

Great job!

Can you sign the DCO? There are instuctions if you click on Details on DCO check.

@semmet95
Copy link
Contributor Author

Hey @FogDong and @charlie0129
Thanks for approving, I'm trying to get the old commits signed, or will probably squash them all.

@semmet95
Copy link
Contributor Author

Hey
So it seems like I made over a 100 commits to my branch and still couldn't get rid of the signed commit check errors 🥹.
Do you guys want me to create a new PR from a new branch with all these changes?

@charlie0129
Copy link
Member

charlie0129 commented May 10, 2023

You can use force-pushing to get rid of the past (unsigned) commits. IMO, continue on this branch is fine.

You can probably click on this to get some help.

image

@semmet95
Copy link
Contributor Author

I tried following the DCO fix guide but I'm getting errors referring to signatures from other contributers
image

I checked out the PR following the linked guide and it seems I pulled all the changes from the PR in my branch 😕

@charlie0129
Copy link
Member

charlie0129 commented May 11, 2023

You can use git reset --hard 69bc6f6 to clean up all the irrelevant commits (make sure you have some kind of backup because it is a hard reset). Then git push -f to force push to your branch.

With your branch back to the original state, then you can solve the DCO.

@semmet95 semmet95 force-pushed the fix/example-manager-and-instructions branch from 0a617ff to 69bc6f6 Compare May 11, 2023 18:15
Somefive and others added 8 commits May 11, 2023 23:47
* Feat: upgrade k8s.io dependency to 0.26

Signed-off-by: Yin Da <[email protected]>

* Fix: golangci-lint action failure

Signed-off-by: Yin Da <[email protected]>

---------

Signed-off-by: Yin Da <[email protected]>
Signed-off-by: Amit Singh <[email protected]>
Feat: add cluster info in action context

Signed-off-by: yangsoon <[email protected]>
Co-authored-by: yangsoon <[email protected]>
Signed-off-by: Amit Singh <[email protected]>
* Refactor: allow the use of singleton for one Source, while also allowing non-singleton Sources, making it more extensible

Signed-off-by: Charlie Chiang <[email protected]>

* Feat: add cronjob source

Signed-off-by: Charlie Chiang <[email protected]>

* Chore: update cron lib

Signed-off-by: Charlie Chiang <[email protected]>

* Feat: do not exit if config contains invalid sources

Signed-off-by: Charlie Chiang <[email protected]>

* Chore: add tests and fix linter

Signed-off-by: Charlie Chiang <[email protected]>

* Chore: go mod tidy

Signed-off-by: Charlie Chiang <[email protected]>

* Docs: add cronjob example

Signed-off-by: Charlie Chiang <[email protected]>

* fix comments

Signed-off-by: Charlie Chiang <[email protected]>

* Chore: force lint to use the exact version

Signed-off-by: Charlie Chiang <[email protected]>

* remove unnecessary dep

Signed-off-by: Charlie Chiang <[email protected]>

* fix comments

Signed-off-by: Charlie Chiang <[email protected]>

* use codecov token

Signed-off-by: Charlie Chiang <[email protected]>

* update lint script

Signed-off-by: Charlie Chiang <[email protected]>

---------

Signed-off-by: Charlie Chiang <[email protected]>
Signed-off-by: Amit Singh <[email protected]>
Signed-off-by: yangsoon <[email protected]>
Co-authored-by: yangsoon <[email protected]>
Signed-off-by: Amit Singh <[email protected]>
Signed-off-by: Amit Singh <[email protected]>
@semmet95 semmet95 force-pushed the fix/example-manager-and-instructions branch from 69bc6f6 to 3d37b0d Compare May 11, 2023 18:17
@semmet95
Copy link
Contributor Author

Thanks a lot @charlie0129
The DCO check is finally passing now

@charlie0129
Copy link
Member

The code changes doesn't seem right. It contains many unrelated changes.

@semmet95
Copy link
Contributor Author

Can I reset back to commit b76f491?
That should remove the unrelated changes.

@semmet95 semmet95 force-pushed the fix/example-manager-and-instructions branch from 3d37b0d to b76f491 Compare May 17, 2023 16:13
This reverts commit 4351008.

Removing changes unrelated to this branch

Signed-off-by: Amit Singh <[email protected]>
This reverts commit 46f5975.

Removes changes unrelated to the branch

Signed-off-by: Amit Singh <[email protected]>
This reverts commit e214be5.

Removes changes unrelated to the branch

Signed-off-by: Amit Singh <[email protected]>
This reverts commit a22e316.

Removes changes unrelated to the branch

Signed-off-by: Amit Singh <[email protected]>
@semmet95 semmet95 force-pushed the fix/example-manager-and-instructions branch from 4895b15 to b358fd1 Compare May 19, 2023 04:12
@semmet95
Copy link
Contributor Author

@charlie0129 I've removed all the unrelated changes and all the commits are signed too.

@FogDong
Copy link
Member

FogDong commented May 19, 2023

Great work! Thanks @semmet95 !

@FogDong FogDong merged commit ed3b9a6 into kubevela:main May 19, 2023
@semmet95 semmet95 deleted the fix/example-manager-and-instructions branch June 2, 2023 12:15
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.

Add api group to clusterrole's first rule in config/manager
5 participants