radish-bdd / radish

Behavior Driven Development tooling for Python. The root from red to green.
https://radish-bdd.github.io
MIT License
182 stars 49 forks source link

Adding Support for filtering by tag with variables. #457

Closed inquisitev closed 1 year ago

inquisitev commented 1 year ago

This will require an update to tag-expressions

requires: https://github.com/timofurrer/tag-expressions/pull/2 to avoid parsing exception

as for the change log, i see an unreleased section, so i am assuming that not every change gets a release. is this how it works in practice? should this get a new version? and if the unreleased section is never moved, may i remove it to reduce confusion?

closes https://github.com/radish-bdd/radish/issues/453

fliiiix commented 1 year ago

nice i try to release this somewhere this week i need to figure out why the github actions do not run and review the change

And for releasing you could have added your feature just to the unreleased section and then at somepoint i would create a new release that would be the general idea but since you also update the version already we can also take that :+1:

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (d08899b) 87.26% compared to head (af6b8ec) 87.26%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #457 +/- ## ======================================= Coverage 87.26% 87.26% ======================================= Files 39 39 Lines 2380 2380 ======================================= Hits 2077 2077 Misses 303 303 ``` | [Flag](https://app.codecov.io/gh/radish-bdd/radish/pull/457/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/radish-bdd/radish/pull/457/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd) | `87.26% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/radish-bdd/radish/pull/457?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd) | Coverage Δ | | |---|---|---| | [radish/hookregistry.py](https://app.codecov.io/gh/radish-bdd/radish/pull/457?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd#diff-cmFkaXNoL2hvb2tyZWdpc3RyeS5weQ==) | `100.00% <100.00%> (ø)` | | | [radish/parser.py](https://app.codecov.io/gh/radish-bdd/radish/pull/457?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=radish-bdd#diff-cmFkaXNoL3BhcnNlci5weQ==) | `99.14% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fliiiix commented 1 year ago

I try to play around with it a bit so i can merge it on the weekend

inquisitev commented 1 year ago

lgtm so there is still the question if we can filter for @tag value somehow otherwise i think that should be more or less ready to be merged

It would likely require replacing all white space with some other character before replacing the parens in tag expressions.

fliiiix commented 1 year ago

@inquisitev please take a look i update the testing a bit and extend the docs and took the liberty to restructure that as a single commit without the release since that should happen from main branch

Let me know if i missed anything and if the updated docs make sense to you.

Minor side-note:

inquisitev commented 1 year ago

@inquisitev please take a look i update the testing a bit and extend the docs and took the liberty to restructure that as a single commit without the release since that should happen from main branch

Let me know if i missed anything and if the updated docs make sense to you.

Minor side-note:

  • is there a reason that you used merges instead of rebases?
  • I think your name contains a extra space character between firstname and lastname Trevor Keegan image I assume thats not on purpose so you might take a look at your git config

Looks good to me.

"is there a reason that you used merges instead of rebases?" - I'm not sure why its using merge, i think i squashed earlier (if i did i used git rebase -i HEAD~n). manually squashing is new to me as we just let github squash on pr merge at work.

"I think your name contains a extra space ..." - thanks for catching that, ive updated my config. whoops