Minor housekeeping work #310
Conversation
There was a problem hiding this comment.
@barafael thanks for taking a look and giving a care. However, most of these changes seem not to be better. I've commented each one of them.
Anything other is OK, and I'm going to merge.
Also, I've added linting without features to CI.
Cargo.toml
Outdated
| "rt-multi-thread", | ||
| "sync", | ||
| "time", | ||
| ] } |
There was a problem hiding this comment.
@barafael I've found it more comfortable to keep each dependency as a single line in [dependencies] section.
codegen/src/attribute.rs
Outdated
| @@ -53,7 +53,7 @@ struct Step { | |||
| /// reference. | |||
| /// | |||
| /// [`gherkin::Step`]: https://bit.ly/3j42hcd | |||
| step_arg_name: Option<syn::Ident>, | |||
| arg_name: Option<syn::Ident>, | |||
There was a problem hiding this comment.
@barafael step_ prefix was referring to a gherkin::Step here, rather than this struct itself.
src/writer/normalize.rs
Outdated
| @@ -489,7 +489,7 @@ impl<World> CucumberQueue<World> { | |||
| /// [`Feature`]: gherkin::Feature | |||
| fn new_feature(&mut self, feat: Event<Arc<gherkin::Feature>>) { | |||
| let (feat, meta) = feat.split(); | |||
| drop(self.queue.insert(feat, FeatureQueue::new(meta))); | |||
| drop(self.fifo.insert(feat, FeatureQueue::new(meta))); | |||
There was a problem hiding this comment.
@barafael not a fan of this particular change. It's more meaningful to emphasize we're queuing something here, rather than bothering reader with exact semantics of the queue, which is "implementation detail".
src/event.rs
Outdated
| @@ -54,7 +54,7 @@ pub struct Event<T: ?Sized> { | |||
| impl<T> Event<T> { | |||
| /// Creates a new [`Event`] out of the given `value`. | |||
| #[must_use] | |||
| pub fn new(value: T) -> Self { | |||
| pub const fn new(value: T) -> Self { | |||
| Self { | |||
| #[cfg(feature = "timestamps")] | |||
| at: SystemTime::now(), | |||
There was a problem hiding this comment.
@barafael this won't compile with --feature timestamps:
error[E0015]: cannot call non-const fn `std::time::SystemTime::now` in constant functions
--> src/event.rs:60:17
|
60 | at: SystemTime::now(),
| ^^^^^^^^^^^^^^^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
| @@ -1485,7 +1485,7 @@ where | |||
| (fut, span_id) | |||
| }; | |||
| #[cfg(not(feature = "tracing"))] | |||
| let _ = scenario_id; | |||
| let _: ScenarioId = scenario_id; | |||
There was a problem hiding this comment.
@barafael I would prefer _ = scenario_id;, but yes:
error[E0658]: attributes on expressions are experimental
--> src/runner/basic.rs:1076:13
|
1076 | #[cfg(not(feature = "tracing"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
tyranron
added
enhancement
I study how people use clippy. I found it interesting that this crate actually sets clippy to pedantic by default. There were minor findings, let me know what you think.