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

🐛 Add hack/tools to bed linted and fix errors in deploy-cli #2123

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

mquhuy
Copy link
Member

@mquhuy mquhuy commented Dec 12, 2024

What this PR does / why we need it:

This pr fixes errors found by osv-scanner in deploy-cli.
It also adds hack/tools package as a linting target, so that
any failure due to dependencies changes will get detected in the future.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2121

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 12, 2024
@mquhuy
Copy link
Member Author

mquhuy commented Dec 12, 2024

/cc @tuminoid @lentzi90 @kashifest

@tuminoid
Copy link
Member

Can you also make it part of linting targets, so it does not get broken again?

Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

Even though you have linked the related issue in the PR message, that won't be visible in the git history as you have no explanation in the commit message.

Please add a short explanation of the issue and the solution to the commit message.

Rozzii
Rozzii previously requested changes Dec 13, 2024
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

Please update the commit message.

@mquhuy mquhuy force-pushed the mquhuy/fix-deploy-cli branch from 4ba5f52 to f6bf2bd Compare December 13, 2024 09:45
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 13, 2024
@mquhuy mquhuy requested a review from Rozzii December 13, 2024 09:59
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Looks good. Is there any test we can trigger here that would actually make use of the deploy-cli?
/approve
/hold

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2024
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

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:

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2024
@Rozzii Rozzii dismissed their stale review December 16, 2024 07:10

It is done.

Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2024
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

Missing GH workflow matrix addition.
/hold

@tuminoid
Copy link
Member

/retitle 🐛 Add hack/tools to bed linted and fix errors in deploy-cli

@metal3-io-bot metal3-io-bot changed the title 🐛 Fix error on Deploy-cli 🐛 Add hack/tools to bed linted and fix errors in deploy-cli Dec 16, 2024
@mquhuy mquhuy force-pushed the mquhuy/fix-deploy-cli branch from f6bf2bd to cf47f48 Compare December 16, 2024 09:07
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2024
This commit fixes errors found by osv-scanner in deploy-cli [1]

It also adds `hack/tools` package as a linting target, so that
any failure due to dependencies changes will get detected in the future.

[1] metal3-io#2121

Signed-off-by: Huy Mai <huy.mai@est.tech>
@mquhuy mquhuy force-pushed the mquhuy/fix-deploy-cli branch from cf47f48 to 25f0c32 Compare December 16, 2024 09:07
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2024
@tuminoid
Copy link
Member

/unhold

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 16, 2024
@metal3-io-bot metal3-io-bot merged commit 7e72f80 into metal3-io:main Dec 16, 2024
18 checks passed
@metal3-io-bot metal3-io-bot deleted the mquhuy/fix-deploy-cli branch December 16, 2024 10:01
@metal3-io-bot metal3-io-bot added this to the BMO - v0.9 milestone Dec 16, 2024
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. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deploy-cli seems rotten and breaks govulncheck
5 participants