Update to edition 2021, lint fixes, and other general code improvements #84

VisualEhrmanntraut · 2022-08-26T16:29:47Z

Improvements include, but are not limited to, fixed typos and grammar, and similar

VisualEhrmanntraut · 2022-08-26T16:48:30Z

I'm still getting from my usage:

error: binding to `_` prefixed variable with no side-effect
  --> src/tables/madt/ic/ioapic.rs:50:5
   |
50 |     pub vector: u8,
   |     ^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding

And I have no idea why

VisualEhrmanntraut · 2022-08-26T16:51:38Z

All good now on my part

VisualEhrmanntraut · 2022-08-27T10:07:36Z

Okay, now it's actually 100% ready to be merged

Robbepop

Thanks a lot for the spelling fixes, edition upgrade, formatting updates and general changes.
I bet this was a lot of work! Many people and projects are depending on this old crate and it could seriously need some love.

The problem I am having with the PR is that it is not easy to review in its current form since many different kinds of works have been intertwined.
While scimming over the PR I saw a few problems, too, such as replacing ::core::primitves::usize with just usize which is bad for proc. macro hygiene.

I currently do not really maintain this crate and I really feel sorry for all the unreviewed PRs and improvements at the moment. I may come back to this project but at the moment I do not have enough free time to work on it.
If you feel like you want to improve this project and are still in need of an interesting side project to work on I'd propose you to fork this repo and implement those improvements on your fork. If you succeed and continue improving the crate I will certainly post a recommendation to use your crate instead of this one or even consider transitioning ownership rights to you directly. What do you think?

Robbepop · 2022-08-30T15:48:21Z

.rustfmt.toml

-report_todo = "Always"
-report_fixme = "Always"


Why remove this?

These keys have been removed according to rustfmt

See here: rust-lang/rustfmt#5101

Robbepop · 2022-08-30T15:49:02Z

impl/src/bitfield/config.rs

-    pub bytes: Option<ConfigValue<usize>>,
-    pub bits: Option<ConfigValue<usize>>,
-    pub filled: Option<ConfigValue<bool>>,
-    pub repr: Option<ConfigValue<ReprKind>>,
-    pub derive_debug: Option<ConfigValue<()>>,
-    pub derive_specifier: Option<ConfigValue<()>>,
+    pub bytes: Option<Value<usize>>,
+    pub bits: Option<Value<usize>>,
+    pub filled: Option<Value<bool>>,
+    pub repr: Option<Value<ReprKind>>,
+    pub derive_debug: Option<Value<()>>,
+    pub derive_specifier: Option<Value<()>>,


Those renamings are very subjective. I personally liked ConfigValue more since it is more on point.

To be honest, me too, but clippy was complaining

VisualEhrmanntraut · 2022-08-30T16:20:02Z

Thanks a lot for the spelling fixes, edition upgrade, formatting updates and general changes. I bet this was a lot of work! Many people and projects are depending on this old crate and it could seriously need some love.

The problem I am having with the PR is that it is not easy to review in its current form since many different kinds of works have been intertwined. While scimming over the PR I saw a few problems, too, such as replacing ::core::primitves::usize with just usize which is bad for proc. macro hygiene.

I currently do not really maintain this crate and I really feel sorry for all the unreviewed PRs and improvements at the moment. I may come back to this project but at the moment I do not have enough free time to work on it. If you feel like you want to improve this project and are still in need of an interesting side project to work on I'd propose you to fork this repo and implement those improvements on your fork. If you succeed and continue improving the crate I will certainly post a recommendation to use your crate instead of this one or even consider transitioning ownership rights to you directly. What do you think?

Personally, if I were to own such bitfield crate, I wouldn't write it the way it's currently written. I keep finding more and more lints to fix and generally had a lot of trouble navigating the source code of this crate

Robbepop · 2022-08-30T16:45:00Z

Thanks a lot for the spelling fixes, edition upgrade, formatting updates and general changes. I bet this was a lot of work! Many people and projects are depending on this old crate and it could seriously need some love.
The problem I am having with the PR is that it is not easy to review in its current form since many different kinds of works have been intertwined. While scimming over the PR I saw a few problems, too, such as replacing ::core::primitves::usize with just usize which is bad for proc. macro hygiene.
I currently do not really maintain this crate and I really feel sorry for all the unreviewed PRs and improvements at the moment. I may come back to this project but at the moment I do not have enough free time to work on it. If you feel like you want to improve this project and are still in need of an interesting side project to work on I'd propose you to fork this repo and implement those improvements on your fork. If you succeed and continue improving the crate I will certainly post a recommendation to use your crate instead of this one or even consider transitioning ownership rights to you directly. What do you think?

Personally, if I were to own such bitfield crate, I wouldn't write it the way it's currently written. I keep finding more and more lints to fix and generally had a lot of trouble navigating the source code of this crate

Yeah it is an old codebase already. If you are looking for a nice side project to work on this might be a good chance to own your own fork or implementation (no need for fork if you rewrite everything anyways) and develop the space of Rust bitfields further for the ecosystem. :)

VisualEhrmanntraut · 2022-08-30T17:05:52Z

Thanks a lot for the spelling fixes, edition upgrade, formatting updates and general changes. I bet this was a lot of work! Many people and projects are depending on this old crate and it could seriously need some love.
The problem I am having with the PR is that it is not easy to review in its current form since many different kinds of works have been intertwined. While scimming over the PR I saw a few problems, too, such as replacing ::core::primitves::usize with just usize which is bad for proc. macro hygiene.
I currently do not really maintain this crate and I really feel sorry for all the unreviewed PRs and improvements at the moment. I may come back to this project but at the moment I do not have enough free time to work on it. If you feel like you want to improve this project and are still in need of an interesting side project to work on I'd propose you to fork this repo and implement those improvements on your fork. If you succeed and continue improving the crate I will certainly post a recommendation to use your crate instead of this one or even consider transitioning ownership rights to you directly. What do you think?

Personally, if I were to own such bitfield crate, I wouldn't write it the way it's currently written. I keep finding more and more lints to fix and generally had a lot of trouble navigating the source code of this crate

Yeah it is an old codebase already. If you are looking for a nice side project to work on this might be a good chance to own your own fork or implementation (no need for fork if you rewrite everything anyways) and develop the space of Rust bitfields further for the ecosystem. :)

Yeah, I'm going to write a new crate, I just needed to fix this one temporarily, and thought I'd contribute these changes for anyone that uses this too

EDIT: More warnings lol

VisualEhrmanntraut added 4 commits August 26, 2022 15:38

Pedantic clippy

4104bc2

Fix a bunch of typos and grammar

741eb72

Fix code error

b82307b

Fix all lints

d8875c5

VisualEhrmanntraut marked this pull request as draft August 26, 2022 16:36

Fix associated member name _

55f676a

Fixed lint clippy::no_effect_underscore_binding

2126abd

VisualEhrmanntraut marked this pull request as ready for review August 26, 2022 16:51

VisualEhrmanntraut added 2 commits August 26, 2022 21:54

Fix trailing semicolon

838269c

Update rustfmt.toml

319408b

VisualEhrmanntraut marked this pull request as draft August 27, 2022 08:52

VisualEhrmanntraut added 4 commits August 27, 2022 12:07

Fix logic and a bunch of warnings

f2f233e

Fix the last chunk of warnings

fb4f338

Fix generate_enum

21e1ab6

Bless test output

da10f56

VisualEhrmanntraut marked this pull request as ready for review August 27, 2022 10:07

Fixed more warnings

3645603

Robbepop reviewed Aug 30, 2022

View reviewed changes

VisualEhrmanntraut closed this by deleting the head repository May 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to edition 2021, lint fixes, and other general code improvements #84

Update to edition 2021, lint fixes, and other general code improvements #84

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 27, 2022

Robbepop left a comment •

edited

Loading

Robbepop Aug 30, 2022

VisualEhrmanntraut Aug 30, 2022

VisualEhrmanntraut Aug 30, 2022

Robbepop Aug 30, 2022

VisualEhrmanntraut Aug 30, 2022

VisualEhrmanntraut commented Aug 30, 2022

Robbepop commented Aug 30, 2022 •

edited

Loading

VisualEhrmanntraut commented Aug 30, 2022 •

edited

Loading

		report_todo = "Always"
		report_fixme = "Always"

Update to edition 2021, lint fixes, and other general code improvements #84

Update to edition 2021, lint fixes, and other general code improvements #84

Conversation

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 26, 2022

VisualEhrmanntraut commented Aug 27, 2022

Robbepop left a comment • edited Loading

Choose a reason for hiding this comment

Robbepop Aug 30, 2022

Choose a reason for hiding this comment

VisualEhrmanntraut Aug 30, 2022

Choose a reason for hiding this comment

VisualEhrmanntraut Aug 30, 2022

Choose a reason for hiding this comment

Robbepop Aug 30, 2022

Choose a reason for hiding this comment

VisualEhrmanntraut Aug 30, 2022

Choose a reason for hiding this comment

VisualEhrmanntraut commented Aug 30, 2022

Robbepop commented Aug 30, 2022 • edited Loading

VisualEhrmanntraut commented Aug 30, 2022 • edited Loading

Robbepop left a comment •

edited

Loading

Robbepop commented Aug 30, 2022 •

edited

Loading

VisualEhrmanntraut commented Aug 30, 2022 •

edited

Loading