Skip to content

Allow unknown lints#279

Closed
tcharding wants to merge 1 commit intorust-bitcoin:masterfrom
tcharding:unknown-lint
Closed

Allow unknown lints#279
tcharding wants to merge 1 commit intorust-bitcoin:masterfrom
tcharding:unknown-lint

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Nov 3, 2021

We support many versions of rustc by design, this puts us in the following predicament:

  1. Linting with a newer version of clippy throws a warning for a newly introduced default lint.
  2. In order to allow the lint we must use the lint by name but this lint does not exist in earlier versions of clippy, so we get a warning unknown lint when linting with the earlier version.

We want to have our cake and eat it too, we can do this by allowing unknown lints. Doing so enables (1) while preventing (2).

As demonstration cargo +1.36 check emits:

warning: unknown lint: `broken_intra_doc_links`

With this patch applied there is no such warning emitted.

The change in this PR has also been submitted in bitcoin_hashes, for brevity it is likely easier if we discuss the failings/merits of this work in a single place on that PR. Thanks.

@tcharding
Copy link
Copy Markdown
Member Author

I'm confused as to why CI is failing for a rust fmt command, I did not think we used rustfmt in the rust-bitcoin stack yet?

@sanket1729
Copy link
Copy Markdown
Member

@tcharding , rustfmt enforced for rust-miniscript from the birth of the project. It was easy to do for rust-miniscript because the project was majorly in late 2019, where rustfmt was stable. rust-bitcoin is a much older project (I think pre rust 1.0! ), so it did not have rustfmt since the get-go and adopting it at a later stage became a task.

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Nov 4, 2021

Oh cool, thanks for the explanation @sanket1729!

@tcharding tcharding force-pushed the unknown-lint branch 2 times, most recently from 6170f51 to 5f27459 Compare November 4, 2021 22:20
We support many versions of rustc by design, this puts us in the
following predicament:

1. Linting with a newer version of clippy throws a warning for a newly
introduced default lint.
2. In order to allow the lint we must use the lint by name but this lint
does not exist in earlier versions of clippy, so we get a warning
`unknown lint` when linting with the earlier version.

We want to have our cake and eat it too, we can do this by allowing
unknown lints. Doing so enables (1) while preventing (2).

To demonstrate this use case `cargo +1.36 check` emits:

  warning: unknown lint: `broken_intra_doc_links`

With this patch applied there is no such warning emitted.
@tcharding
Copy link
Copy Markdown
Member Author

Based off comments in the same PR as this on rust-bitcoin, this PR is not useful. Please excuse the noise.

@tcharding tcharding closed this Nov 5, 2021
@tcharding tcharding deleted the unknown-lint branch May 6, 2022 01:13
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