Minor linting, warning fixes #84
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Aug 11, 2023
* Fix `cargo test --no-default-features` to pass by making std-only tests/docs conditional (added to CI) * inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints * Added `Error::Custom(&'static str)` for no-std mode There are a few `FIXME` in the code where things seemed suspicious
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Aug 11, 2023
* Fix `cargo test --no-default-features` to pass by making std-only tests/docs conditional (added to CI) * inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints * Added `Error::Custom(&'static str)` for no-std mode There are a few `FIXME` in the code where things seemed suspicious
|
@nyurik i'm very sorry I missed/dropped this PR, I don't know if you're still interested, but getting nostd tests passing is definitely something worthwhile. If you are still interested, I think splitting out just getting nostd tests passing will be useful (and easier to review), and then if you like, fixing various lints and other things as a separate PR. |
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Nov 5, 2023
* inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints There are a few `FIXME` in the code where things seemed suspicious
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Nov 5, 2023
* inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints There are a few `FIXME` in the code where things seemed suspicious
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Nov 5, 2023
* inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints There are a few `FIXME` in the code where things seemed suspicious
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Nov 5, 2023
* inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints There are a few `FIXME` in the code where things seemed suspicious
nyurik
added a commit
to nyurik/scroll
that referenced
this pull request
Nov 6, 2023
* inline format strings, and optimize their usage (using `&arg` with format causes a bit of a performance delay) * Add MSRV to Cargo.toml * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints There are a few `FIXME` in the code where things seemed suspicious
nyurik
force-pushed
the
cleanup
branch
2 times, most recently
from
3bf7506 to
e1a5d6a
Compare
nyurik
changed the title
nyurik
force-pushed
the
cleanup
branch
2 times, most recently
from
fac343c to
330ab9d
Compare
|
@m4b hi, is there anything else that should be fixed in this PR? Thx! |
m4b
reviewed
Nov 13, 2023
| @@ -1,14 +1,7 @@ | |||
| // this exists primarily to test various API usages of scroll; e.g., must compile | |||
There was a problem hiding this comment.
i don't think this comment needed to be deleted
* Optimize/cleanup use statements by using `cargo +nightly fmt -- --config imports_granularity=Module,group_imports=StdExternalCrate` * Do not expose `scroll_derive` as a feature in Cargo.toml * Minor clippy lints - [ ] There is a FIXME in the code that needs to be addressed before merging
|
@m4b thx, replied. Could you take a look at https://github.com/m4b/scroll/pull/84/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R273-R274 -- the test in src/lib.rs -- is |
|
You could try removing the allow and see what happens? I can't recall what it's there, it was probably added years ago for some reason lost to time :) |
|
@m4b good point, removed, seems fine :) |
m4b
approved these changes
Nov 13, 2023
|
its not thankless if its gets merged :) Thx for working on this! |
m4b
pushed a commit
that referenced
this pull request
Dec 31, 2023
* Minor linting * Optimize/cleanup use statements by using `cargo +nightly fmt -- --config imports_granularity=Module,group_imports=StdExternalCrate` * Do not expose `scroll_derive` as a feature in Cargo.toml * removing unneeded allow
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
scroll_deriveas a feature in Cargo.tomlThe format args inlining was moved to #91