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

Enhance CI workflow with additional checks and build steps #10

Closed
wants to merge 4 commits into from
Closed

Enhance CI workflow with additional checks and build steps #10

wants to merge 4 commits into from

Conversation

DharunKumar04
Copy link

Hi @AlexsJones ,

This Pull Request enhances the existing CI workflow by introducing several new checks and build steps:

  • Installing cargo-audit to check for vulnerabilities
  • Running cargo-fmt to format code
  • Running cargo clippy to detect potential linting errors
  • Running cargo check to check for syntax errors and other compilation issues

These changes will help to ensure that our code is of high quality and is free of security vulnerabilities.

PS: For enhanced code comprehension, I've incorporated the cargo doc command into the CI stage to generate comprehensive documentation. If this approach is satisfactory, we can publish this documentation to crates.io

@AlexsJones
Copy link
Contributor

Awesome suggestion @DharunKumar04
Whatr are your thoughts about this error:

Run cargo fmt --all -- --check
error: 'cargo-fmt' is not installed for the toolchain 'nightly-x8[6](https://github.com/isotope-rs/isotope/actions/runs/6906753713/job/18796567820?pr=10#step:6:7)_64-unknown-linux-gnu'
To install, run `rustup component add rustfmt`

@DharunKumar04
Copy link
Author

Hi @AlexsJones ,

I apologize for the error message you encountered when running cargo fmt --all -- --check. I overlooked the fact that cargo-fmt is not preinstalled in the nightly toolchain. To resolve this issue, I will add the rustfmt component to the toolchain. I will also add clippy to the toolchain for additional code analysis. I will include these changes in the next commit. Please rerun the cargo fmt command after the changes have been committed.

Using tools like rustfmt and clippy can help to ensure that our codebase is clean, readable, and consistent. This can make it easier for developers to understand and contribute to the codebase in the future. Additionally, these tools can help to catch potential bugs and errors before they cause problems.

@AlexsJones
Copy link
Contributor

Hi @AlexsJones ,

I apologize for the error message you encountered when running cargo fmt --all -- --check. I overlooked the fact that cargo-fmt is not preinstalled in the nightly toolchain. To resolve this issue, I will add the rustfmt component to the toolchain. I will also add clippy to the toolchain for additional code analysis. I will include these changes in the next commit. Please rerun the cargo fmt command after the changes have been committed.

Using tools like rustfmt and clippy can help to ensure that our codebase is clean, readable, and consistent. This can make it easier for developers to understand and contribute to the codebase in the future. Additionally, these tools can help to catch potential bugs and errors before they cause problems.

This is fun because it looks like you generated this reponse and PR from chatgpt, but Im not even mad 👯‍♂️

- name: Run Clippy linter
run: |
rustup component add clippy
cargo clippy -- -D warnings
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue here is that it is going to force some areas in the code to fail this warning, but functionally they are designed that way for a reason

Copy link
Author

Choose a reason for hiding this comment

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

please give me some time alex i will work on this

Copy link
Author

Choose a reason for hiding this comment

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

@AlexsJones i used this command cargo clippy -- -D warnings to deny all the lint errors that will stop the workflow hoping it would make the codebase clean . But i get your point, some things are designed that way for a specific reason right .

So instead of denying all the lint errors we can modify it in such a way that it will only denies specific lint using the command cargo clippy -- -A clippy::all -D clippy::absurd_extreme_comparisons. But i think whitelisting each lints by name is a time consuming task and it wont be effective . So for this, we can make use of the lint groups that are available in clippy but even still this might be a roadblock for devs because it will throw errors .

Kindly share your thoughts on this whether we can remove this clippy feature or we can whilelist particular lints (by name or by group) and i will make the necessary changes alex.

PS : for your reference list of lints available in clippy

@AlexsJones AlexsJones closed this Jan 4, 2024
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.

2 participants