Skip to content
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

Custom Parameters in Cucumber Expressions (#124) #168

Merged
merged 15 commits into from
Nov 29, 2021

Conversation

ilslv
Copy link
Member

@ilslv ilslv commented Nov 26, 2021

Part of #124

Synopsis

For now there is only support for default parameters in cucumber expressions.

Solution

Implement Parameter derive macro and add const asserts to avoid runtime panics.

Checklist

  • Created PR:
    • In draft mode
    • Name contains Draft: prefix
    • Name contains issue reference
    • Has assignee
  • Documentation is updated (if required)
  • Tests are updated (if required)
  • Changes conform code style
  • CHANGELOG entry is added (if required)
  • FCM (final commit message) is posted
    • and approved
  • Review is completed and changes are approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • Draft: prefix is removed
    • All temporary labels are removed

@ilslv ilslv added the enhancement Improvement of existing features or bugfix label Nov 26, 2021
@ilslv ilslv added this to the 0.11 milestone Nov 26, 2021
@ilslv ilslv self-assigned this Nov 26, 2021
@ilslv
Copy link
Member Author

ilslv commented Nov 29, 2021

FCM

Implement `Parameter` derive macro for Cucumber Expressions (#168, #124)

@ilslv ilslv marked this pull request as ready for review November 29, 2021 08:09
@ilslv ilslv changed the title Draft: Custom Parameters in Cucumber Expressions (#124) Custom Parameters in Cucumber Expressions (#124) Nov 29, 2021
@ilslv ilslv requested a review from tyranron November 29, 2021 08:09
Cargo.toml Outdated
@@ -40,7 +40,7 @@ timestamps = []
async-trait = "0.1.40"
atty = "0.2.14"
console = "0.15"
derive_more = { version = "0.99.16", features = ["as_ref", "deref", "deref_mut", "display", "error", "from", "into"], default_features = false }
derive_more = { version = "0.99.16", features = ["as_ref", "deref", "deref_mut", "display", "error", "from", "from_str", "into"], default_features = false }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't use derive(FromStr) in src/, we shouldn't introduce it here, but rather in [dev-dependencies].

@@ -54,6 +54,7 @@ structopt = "0.3.25"

# "macros" feature dependencies
cucumber-codegen = { version = "0.11.0-dev", path = "./codegen", optional = true }
cucumber-expressions = { version = "0.1.0", features = ["into-regex"], optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, we don't need "into-regex" feature in cucumber crate directly, so we shouldn't require it.

name: Option<syn::LitStr>,
}

/// Representation of a struct implementing `Parameter`, used for code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only struct? Any type satisfying the requirements will be valid here.

Provider,
)
.unwrap_or_else(|e| {
panic!("Cucumber expression failed: {}", e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it should never fail, then we may use just unwrap(), as we do with regular expressions.

@tyranron tyranron merged commit b7e9ff9 into main Nov 29, 2021
@tyranron tyranron deleted the 124-custom-cucumber-expr-parameters branch November 29, 2021 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants