-
Notifications
You must be signed in to change notification settings - Fork 829
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
Define top level Permission for cli.yml workflow #5079
base: master
Are you sure you want to change the base?
Define top level Permission for cli.yml workflow #5079
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5079 +/- ##
==========================================
- Coverage 28.21% 28.20% -0.01%
==========================================
Files 632 632
Lines 43568 43557 -11
==========================================
- Hits 12291 12284 -7
+ Misses 30381 30374 -7
- Partials 896 899 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
.github/workflows/cli.yaml
Outdated
@@ -12,6 +12,9 @@ on: | |||
concurrency: | |||
group: ${{ github.workflow }}-${{ github.actor }}-${{ github.head_ref || github.run_id }} | |||
cancel-in-progress: true | |||
permissions: | |||
contents: read # Required to check out the code | |||
id-token: write # Required for authentication methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id-token: write # Required for authentication methods |
permission id-token: write
is unnecessary
.github/workflows/cli.yaml
Outdated
@@ -54,5 +57,4 @@ jobs: | |||
with: | |||
cluster-resources: nodes,namespaces, | |||
namespace-resources: configmaps,pods,svc | |||
artifact-name: logs-${{ matrix.k8s}} | |||
|
|||
artifact-name: logs-${{ matrix.k8s}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to modify it here
@zhzhuang-zju Your requested changes have been made |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please squash the commits. and then LGTM
@@ -55,4 +57,3 @@ jobs: | |||
cluster-resources: nodes,namespaces, | |||
namespace-resources: configmaps,pods,svc | |||
artifact-name: logs-${{ matrix.k8s}} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not change.
@Akash-Singh04 others LGTM |
39903be
to
207e78b
Compare
Signed-off-by: Akash Singh <akashsingh2210670@gmail.com> Removed excess permissions Signed-off-by: Akash Singh <akashsingh2210670@gmail.com> Removed newline Signed-off-by: Akash Singh <akashsingh2210670@gmail.com> Removed extra line Signed-off-by: Akash Singh <akashsingh2210670@gmail.com>
207e78b
to
8426c23
Compare
@zhzhuang-zju Done |
@@ -55,4 +57,4 @@ jobs: | |||
cluster-resources: nodes,namespaces, | |||
namespace-resources: configmaps,pods,svc | |||
artifact-name: logs-${{ matrix.k8s}} | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Akash-Singh04
Refer to https://gist.github.com/camh-/1bebfcff1b0f814e9b191edc60d5206b, a github text file should end with a new line. The current cli.yaml
file is actually end with two new lines, so you can choose to delete just one of the lines, or leave them alone. What you've done is actually remove a new line and add a line that contains invisible characters
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Which issue(s) this PR fixes:
Part of #5048
Special notes for your reviewer:
Does this PR introduce a user-facing change?: