-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrap event::Cucumber in Event to allow metadata #145
Merged
Merged
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
f031fc2
Add --verbose flag and remove legacy CLI options
ilslv a4a8f62
Add --scenario-name and --scenario-tags CLI options
ilslv f0220bb
Add --features option
ilslv 696de67
Correction
ilslv 7b24407
Add Cucumber methods for additional user CLI
ilslv d9c2589
Correction
ilslv ba2eccf
Merge branch 'main' into 134-cli-redesign
ilslv 0b58059
Corrections
ilslv d7bb724
Corrections
ilslv c48d942
Corrections and add todo_or_die crate
ilslv 4119545
Corrections
ilslv f0c65db
Lint
ilslv 7c47f3b
Try removing todo-or-die crate
ilslv 90cab88
Add Custom generic parameter to cli::Opts
ilslv 2e83a94
Mention CLI in Features chapter of the book
ilslv d104147
Bump version to 0.11 and add CHANGELOG entry
ilslv 75f542a
Some corrections [skip ci]
tyranron cf8e669
Corrections
ilslv d345c4c
Move trait bound from impls to struct in Cucumber
ilslv c2d254c
Corrections
ilslv ff891a9
Pair DateTime with each event
ilslv 598d0bc
Correction
ilslv be20764
Fix chrono at 0.4.19 to avoid problems with minimal-versions
ilslv 57ed9ea
Merge branch 'main' into add-datetime-to-events
tyranron aab2e27
Refactor `(Event, DateTime<Utc>)` into `DateTimed<Event>`
ilslv 0f4d2ea
Wrap `event::Cucumber` in `Event<T>`
ilslv 7808d76
Correction
ilslv 39ba0df
Corrections [run ci]
tyranron 3c4e144
Add changelog entry
tyranron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, this is not very good decision, now we have the code base polluted with those tuples and signatures became worse.
I see here two ways:
DateTime
directly toevent
s types and errors.(T, DateTime)
pairs haveDateTimed<T>
wrapper, which encapsulates the stuff and provides obvious interface to inner values.With the option 2, we will need to only change type signatures, without touching any code. Furthermore, this way we can hide it completely behind a feature gate, so it can be used only for those
Writer
s which really need it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tyranron
As all events are nested enums, we'll have to put
DateTime
in every variant of theCucumber
eventMatching over it will be very painful.
👍
Not really, especially in case of
writer::Normalized
. There we store onlyevent::Step
and path to it:Feature
,Option<Rule>
,Scenario
. Then we rebuild it withCucumber::scenario(feat, rule, sc, ev)
when it's time to emit it. But it's pretty easy to storeDateTime
s of those events and already done in this PR.I'm not sure we should. This will cause
Writer
trait to be different in cases when feature is on or off (same forRunner
)