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 formatting and add cargo fmt to CI #23

Open
Samyak2 opened this issue Apr 30, 2022 · 4 comments
Open

Fix formatting and add cargo fmt to CI #23

Samyak2 opened this issue Apr 30, 2022 · 4 comments
Labels
good first issue Good for newcomers

Comments

@Samyak2
Copy link
Collaborator

Samyak2 commented Apr 30, 2022

Some of the code isn't formatted. Can be fixed by running cargo fmt.

We should also enforce formatting by checking for formatting in CI. See this for an example.

@Samyak2 Samyak2 added the good first issue Good for newcomers label Apr 30, 2022
@AugustoFKL
Copy link
Contributor

AugustoFKL commented May 1, 2022

Should be used the nightly feature?

It isn't a common-sense between projects, so I prefer to ask the main maintainer before doing stuff

In my own projects I usually use:

cargo clippy --all-targets --all-features -- -D warnings

and

cargo +nightly fmt --verbose -- --check

What do you think?

@Samyak2
Copy link
Collaborator Author

Samyak2 commented May 2, 2022

cargo clippy --all-targets --all-features -- -D warnings

This seems like a good default. Although, I would like to have a separate issue/PR for clippy. I'll create one now. (Edit: it's #25).

cargo +nightly fmt --verbose -- --check

What's the benefit of using nightly for cargo fmt? Also, what additional information would --verbose give that's needed in CI (the default output of cargo fmt --check lists diffs for files needing formatting, which is all one would need to know about formatting anyway)?

@AugustoFKL
Copy link
Contributor

Sure, will do this in different PR's as requested.

About the nightly, it's not a convention. I use in my projects because, as some say, nightly supports more formatting options and produce prettier results.

But it's unstable. If you prefer to use the stable option (default), that's fine.

The verbose option is a personal pattern as well, will remove from the workflow

@Samyak2
Copy link
Collaborator Author

Samyak2 commented May 8, 2022

About the nightly, it's not a convention. I use in my projects because, as some say, nightly supports more formatting options and produce prettier results.

That makes sense. We should use nightly for spressolisp too.

@Samyak2 Samyak2 mentioned this issue May 10, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants