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 GitHub Actions workflow ci.yaml #34

Merged
merged 7 commits into from
Feb 21, 2024
Merged

Conversation

dongsupark
Copy link
Collaborator

Address compiler warnings given by cargo check or cargo build.

Address coding style issues given by cargo fmt.

Address issues given by linter, cargo clippy.

  • Remove unnecessary return.
  • Remove unnecessary to_string.
  • Remove comparison of bool with true/false.
  • Remove unnecessary references.
  • Remove redundant use tokio.
  • Remove unnecessary let binding.
  • Remove empty strings in println!().
  • Use vec![] macro instead of pushing values to a mutable Vec.
  • Use &str instead of &String to avoid creating a new object.

Add new GitHub Actions workflow ci.yaml, which runs cargo fmt, cargo build, cargo test, and cargo clippy.

Add GitHub CI status banner in README.md.

Testing done

cargo test, cargo fmt, cargo clippy passed.

Address compiler warnings given by `cargo check` or `cargo build`.
Address coding style issues given by `cargo fmt`.
Address issues given by linter, `cargo clippy`.

* remove unnecessary return.
* remove unnecessary to_string.
* remove comparison of bool with true/false
* remove unnecessary reference
* remove redundant use tokio
* remove unnecessary let binding
* remove empty strings in println!()
* use vec![] macro instead of pushing values to a mutable Vec
* use &str instead of &String to avoid creating a new object
@dongsupark dongsupark mentioned this pull request Jan 31, 2024
6 tasks
@dongsupark dongsupark marked this pull request as ready for review January 31, 2024 16:09
Add new GitHub Actions workflow `ci.yaml`, which runs `cargo fmt`,
`cargo build`, `cargo test`, and `cargo clippy`.
@@ -1,5 +1,7 @@
# Azure Provisioning Agent

[![Github CI](https://github.com/Azure/azure-provisioning-agent/actions/workflows/ci.yaml/badge.svg)](https://github.com/Azure/azure-provisioning-agent/actions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some note to README to indicate that all PRs should run cargo fmt, cargo clippy, and cargo tests before submission, otherwise CI will fail and merging will be blocked.
Better yet, we can add some target to the Makefile such as "make tests", "make lint", "make format" to assist with invoking the proper cargo steps during development/submitting PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added notes to CONTRIBUTING.md.
Added makefile targets.

@dongsupark
Copy link
Collaborator Author

Ping, is there anything to be done? Can we merge?

Copy link
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

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

LGTM

@anhvoms anhvoms merged commit d19b788 into main Feb 21, 2024
2 checks passed
@anhvoms anhvoms deleted the dongsu/github-actions-fmt-clippy branch February 21, 2024 16:24
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.

None yet

2 participants