Skip to content

ci: rust checkfmt #2189

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

Merged
merged 1 commit into from
Dec 3, 2019
Merged

ci: rust checkfmt #2189

merged 1 commit into from
Dec 3, 2019

Conversation

ethanyzhang
Copy link
Contributor

Done checklist

  • docs/SPEC.md updated
  • Test cases written

@ethanyzhang ethanyzhang requested a review from affo November 22, 2019 18:09
Copy link
Contributor

@affo affo left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

If you prefer, we can skip formatting those generated files: rust-lang/rustfmt#2522

Basically you can add a rustfmt.toml file with:

ignore = [
    "src/ast/flatbuffers/ast_generated.rs",
    "src/ast/flatbuffers/semantic_generated.rs",
]

For more, see: https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#ignore

I don't request for changes because your solution seems good enough.

@affo affo requested a review from jpacik November 25, 2019 16:20
@jpacik
Copy link
Contributor

jpacik commented Nov 25, 2019

Just to echo what @affo said, I would prefer to ignore generated files, but approving anyway.

@ethanyzhang ethanyzhang force-pushed the ci/rust-checkfmt branch 2 times, most recently from 15641c5 to ab8e585 Compare December 3, 2019 08:07
@ethanyzhang
Copy link
Contributor Author

I tried the ignore option, got this:

Warning: can't set `ignore = IgnoreList { path_set: {"src/ast/flatbuffers/ast_generated.rs", "src/ast/flatbuffers/semantic_generated.rs"}, rustfmt_toml_path: "" }`, unstable features are only available in nightly channel.
Warning: can't set `ignore = IgnoreList { path_set: {"src/ast/flatbuffers/semantic_generated.rs", "src/ast/flatbuffers/ast_generated.rs"}, rustfmt_toml_path: "" }`, unstable features are only available in nightly channel.
Warning: can't set `ignore = IgnoreList { path_set: {"src/ast/flatbuffers/semantic_generated.rs", "src/ast/flatbuffers/ast_generated.rs"}, rustfmt_toml_path: "" }`, unstable features are only available in nightly channel.

Rebased the PR against master

@codecov-io
Copy link

codecov-io commented Dec 3, 2019

Codecov Report

Merging #2189 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2189   +/-   ##
=======================================
  Coverage   53.08%   53.08%           
=======================================
  Files         287      287           
  Lines       39006    39006           
=======================================
  Hits        20705    20705           
  Misses      16281    16281           
  Partials     2020     2020

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30748c2...219e295. Read the comment docs.

@ethanyzhang
Copy link
Contributor Author

In this case, I feel formatting the generated code is OK. Unless you have some strong reasons not to @jlapacik @affo .

@affo
Copy link
Contributor

affo commented Dec 3, 2019

@yzhang1991 Oh, only available in nightly.

Go ahead then 👍

Thank you!

@ethanyzhang ethanyzhang merged commit 556ab98 into master Dec 3, 2019
@ethanyzhang ethanyzhang deleted the ci/rust-checkfmt branch December 3, 2019 16:14
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

4 participants