fix: bump MSRV to 1.71.1 #1119

rnbguy · 2024-03-09T10:46:55Z

Closes: #1118
Closes: #1101
Closes: #1120

Description

Increase MSRV to 1.71.1
Check MSRV on CI for linux and darwin
Update README with new links
Apply some clippy suggestions

PR author checklist:

Added changelog entry, using unclog.
Added tests.
Linked to GitHub issue.
Updated code comments and documentation (e.g., docs/).
Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

Reviewed Files changed in the GitHub PR explorer.
Manually tested (in case integration/unit/mock tests are absent).

codecov · 2024-03-09T10:51:24Z

Codecov Report

Attention: Patch coverage is 64.70588% with 78 lines in your changes are missing coverage. Please review.

Project coverage is 66.54%. Comparing base (e1f19af) to head (6161f2c).

Files	Patch %	Lines
ibc-apps/ics20-transfer/src/module.rs	9.09%	20 Missing ⚠️
ibc-apps/ics721-nft-transfer/src/module.rs	9.52%	19 Missing ⚠️
ibc-core/ics02-client/types/src/error.rs	0.00%	6 Missing ⚠️
...ics24-host/cosmos/src/upgrade_proposal/proposal.rs	0.00%	4 Missing ⚠️
ibc-clients/ics07-tendermint/types/src/error.rs	0.00%	3 Missing ⚠️
ibc-core/ics03-connection/types/src/error.rs	0.00%	3 Missing ⚠️
ibc-derive/src/client_state.rs	70.00%	3 Missing ⚠️
ibc-primitives/src/types/timestamp.rs	88.88%	3 Missing ⚠️
...stkit/src/testapp/ibc/clients/mock/client_state.rs	0.00%	3 Missing ⚠️
...ics07-tendermint/src/client_state/update_client.rs	88.88%	2 Missing ⚠️
... and 11 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1119      +/-   ##
==========================================
+ Coverage   66.51%   66.54%   +0.02%     
==========================================
  Files         209      209              
  Lines       20725    20617     -108     
==========================================
- Hits        13785    13719      -66     
+ Misses       6940     6898      -42

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/rust.yml

ibc-primitives/src/utils/macros.rs

Farhad-Shabani · 2024-03-11T20:42:07Z

ibc-testkit/src/testapp/ibc/core/types.rs

    max_clock_drift: Duration,
-    #[builder(default = Duration::from_secs(128000))]
+    #[builder(default = Duration::from_secs(128_000))]


I liked the tidy outcome from clippy, however, couldn’t catch them on my machine. What version of clippy do you use? Did the CI detect these cases?

As for the particular case of the 128_000, it's strange that 64000 would be considered acceptable.

I cherry picked the lints after running all clippy lint groups. We shouldn't do this directly on CI (but probably locally for reviewing PRs is fine) - as most of them are very opinionated. But we can definitely add a bunch of them on CI.

For the particular case, the lint case is called unreadable_literal. Looks like they warn this only when the decimal has 6 digits i.e. million.

Let's fix things when the linter catches them. That's alright.

ibc-core/ics04-channel/src/handler/acknowledgement.rs

.changelog/unreleased/improvements/1101-use-let-else-over-downcast.md

rnbguy · 2024-03-12T08:00:31Z

btw, I want to fix one more lint but the changes are too big. You can try it by running

cargo clippy --fix -- -W clippy::use_self

* bump msrv to 1.71.1 * add ci check * rm windows * refactor msrv check * use cargo-msrv * add names to steps * apply clippy suggestions * rm redundant import * clippy::use_self * clippy::cast_lossless * clippy::match_same_arms * apply clippy suggestions * reuse old var * refactor old code * badge for license, version, downloads * rm broken tokei loc badge * add changelog * clippy::derive_partial_eq_without_eq * clippy::string_lit_as_bytes * clippy::empty_line_after_doc_comments * clippy::cloned_instead_of_copied * clippy::unreadable_literal * clippy::single_match_else * expect over unwrap * use let-else * use ok_or_else * propagate None * clippy::match_same_arms * return precise error * rm downcast in favor of let-else * chore: add unclog for 1101 + move 1118 under breaking-changes * add msrv in clippy config * update changelog * use into over from * clippy::redundant_closure_for_method_calls --------- Co-authored-by: Farhad Shabani <farhad.shabani@gmail.com>

rnbguy marked this pull request as ready for review March 9, 2024 10:46

rnbguy added 6 commits March 9, 2024 15:38

bump msrv to 1.71.1

dfaa738

add ci check

513594c

rm windows

bbe76bf

refactor msrv check

e022acf

use cargo-msrv

28947f6

add names to steps

44c64d0

rnbguy force-pushed the rano/bump-msrv branch from 364026a to 44c64d0 Compare March 9, 2024 14:38

rnbguy changed the title ~~fix: bump msrv~~ fix: bump MSRV to 1.71.1 Mar 9, 2024

rnbguy added 8 commits March 9, 2024 15:59

apply clippy suggestions

cd44a48

rm redundant import

b640135

clippy::use_self

568a5a2

clippy::cast_lossless

f5c12ff

clippy::match_same_arms

0544df3

apply clippy suggestions

0a1a708

reuse old var

aff9942

refactor old code

fcda368

rnbguy force-pushed the rano/bump-msrv branch from 9be3dc7 to fcda368 Compare March 10, 2024 15:49

rnbguy added 3 commits March 10, 2024 18:05

badge for license, version, downloads

38b5371

rm broken tokei loc badge

665fc15

add changelog

e065529

rnbguy requested a review from Farhad-Shabani March 11, 2024 06:57

rnbguy added 7 commits March 11, 2024 11:17

clippy::derive_partial_eq_without_eq

2084711

clippy::string_lit_as_bytes

ac7c4ab

clippy::empty_line_after_doc_comments

cb32ce8

clippy::cloned_instead_of_copied

604c69e

clippy::unreadable_literal

8587c28

clippy::single_match_else

8ba7ad1

expect over unwrap

e1c14a1

rnbguy added 5 commits March 11, 2024 13:13

use let-else

91d2c68

use ok_or_else

5b221d4

propagate None

0f8001d

clippy::match_same_arms

6acf412

return precise error

b50d08b

rnbguy marked this pull request as draft March 11, 2024 14:26

rm downcast in favor of let-else

4b0f27f

rnbguy marked this pull request as ready for review March 11, 2024 14:34

chore: add unclog for 1101 + move 1118 under breaking-changes

1e90a9b

Farhad-Shabani reviewed Mar 11, 2024

View reviewed changes

Farhad-Shabani added this to the 0.51.0 milestone Mar 11, 2024

Farhad-Shabani reviewed Mar 11, 2024

View reviewed changes

ibc-core/ics04-channel/src/handler/acknowledgement.rs Show resolved Hide resolved

add msrv in clippy config

c7beb71

rnbguy commented Mar 12, 2024

View reviewed changes

.changelog/unreleased/improvements/1101-use-let-else-over-downcast.md Outdated Show resolved Hide resolved

rnbguy added 3 commits March 12, 2024 08:32

update changelog

332c5de

use into over from

d435bda

clippy::redundant_closure_for_method_calls

6161f2c

Farhad-Shabani approved these changes Mar 12, 2024

View reviewed changes

Farhad-Shabani merged commit b2c2529 into main Mar 12, 2024
17 checks passed

Farhad-Shabani deleted the rano/bump-msrv branch March 12, 2024 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: bump MSRV to 1.71.1 #1119

fix: bump MSRV to 1.71.1 #1119

rnbguy commented Mar 9, 2024 •

edited

Loading

codecov bot commented Mar 9, 2024 •

edited

Loading

Farhad-Shabani Mar 11, 2024 •

edited

Loading

rnbguy Mar 12, 2024

rnbguy Mar 12, 2024 •

edited

Loading

Farhad-Shabani Mar 12, 2024

rnbguy commented Mar 12, 2024

fix: bump MSRV to 1.71.1 #1119

fix: bump MSRV to 1.71.1 #1119

Conversation

rnbguy commented Mar 9, 2024 • edited Loading

Description

PR author checklist:

Reviewer checklist:

codecov bot commented Mar 9, 2024 • edited Loading

Codecov Report

Farhad-Shabani Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

rnbguy Mar 12, 2024

Choose a reason for hiding this comment

rnbguy Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Farhad-Shabani Mar 12, 2024

Choose a reason for hiding this comment

rnbguy commented Mar 12, 2024

rnbguy commented Mar 9, 2024 •

edited

Loading

codecov bot commented Mar 9, 2024 •

edited

Loading

Farhad-Shabani Mar 11, 2024 •

edited

Loading

rnbguy Mar 12, 2024 •

edited

Loading