Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Lints #156

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Lints #156

wants to merge 2 commits into from

Conversation

yoshuawuyts
Copy link
Contributor

@yoshuawuyts yoshuawuyts commented Jul 1, 2020

o/ This PR is an attempt to ensure that quaint will continue to compile in future versions of the compiler and introduce a few new code quality measures that will help automate review.

This PR currently fails CI because it turns out that we, in fact, are not future compatible. In particular the issue we're running into is rust-lang/rust#62411, which will likely be turned into an error for the 2021 edition. But beyond that it seems there are a fair number of other issues we may want to address.

Anyway, I found myself having a spare 30 mins today and was curious how this would turn out. I'm opening this as a draft PR to share what's going on, and if this is deemed useful perhaps someone (could be me!) could go through and address the errors and warnings this has surfaced. Thanks!

@@ -24,6 +24,7 @@ docker run -w /build --network test-net -v $BUILDKITE_BUILD_CHECKOUT_PATH:/build
-e TEST_MYSQL=mysql://prisma:prisma@test-mysql:3306/prisma \
-e TEST_PSQL=postgres://prisma:prisma@test-postgres:5432/prisma \
-e TEST_MSSQL="sqlserver://test-mssql:1433;user=SA;password=$MSSQL_SA_PASSWORD;trustServerCertificate=true" \
-e RUSTFLAGS="-Dwarnings" \
Copy link
Contributor Author

@yoshuawuyts yoshuawuyts Jul 1, 2020

Choose a reason for hiding this comment

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

This ensures we don't ship warnings to our main branch, but allows them to exist during local development.

Comment on lines +106 to +110

#![forbid(unsafe_code, future_incompatible, rust_2018_idioms)]
#![deny(missing_debug_implementations, nonstandard_style)]
#![warn(missing_docs, unreachable_pub)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the set of lints introduced. This includes linting for rust_2018_idioms which are things that are disallowed in the current edition and can prevent us from using rust 2018 in the future. And future_incompatible which are things that will be disallowed in a future edition (due next year).

@yoshuawuyts
Copy link
Contributor Author

Talked with @pimeys and some of the current issues we're running into will be resolved with the switch away from tokio-postgres to sqlx. We can pick this patch back up after that!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant