Closed nhatthm closed 2 years ago
I wonder why CircleCI did not start for this PR. 🤔
Merging #424 (afaebf2) into main (afaebf2) will not change coverage. The diff coverage is
n/a
.:exclamation: Current head afaebf2 differs from pull request most recent head 8f61688. Consider uploading reports for the commit 8f61688 to get more accurate results
@@ Coverage Diff @@
## main #424 +/- ##
=======================================
Coverage 82.66% 82.66%
=======================================
Files 26 26
Lines 2533 2533
=======================================
Hits 2094 2094
Misses 341 341
Partials 98 98
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update afaebf2...8f61688. Read the comment docs.
Hi @nhatthm,
Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾
In return for this generous offer we hope you will:
On behalf of the Cucumber core team, Aslak Hellesøy Creator of Cucumber
I wonder why CircleCI did not start for this PR. 🤔
People were using GitHub CI to mine Bitcoin. So now maintainers have to authorize CI for pull requests from first time contributors.
@mpkorstanje thanks for clarification. Could you share how to authorize CI? I worked around the issue by rebasing PR into a local branch, but that felt a bit unnecessary. :)
@mattwynne I can't work out which group @vearutop should be in. I don't think we wrote down any guidelines for that. Could be we add some docs to the groups?
Wait, godog is still on circle ci. I don't know who can trigger that at all. But probably the same kinda permissions problem.
@mpkorstanje would it make sense to migrate godog
pipeline to github actions (I can do that easily)?
Yes. That would make sense. Github actions provides better integration with GitHub and Matt is building release automation based on Github Actions.
As far as permissions goes, what I see is that we have a @cucunmber/go team with Admin rights on this repo. @vearutop is alreayd in that team so I'm not sure there's much more rights we could give him, short of making him an org admin.
@mpkorstanje docs sound like a good idea. What do you think we need to write down, and where?
Description
There is an breaking change in
v0.12.0
, thebeforeStep
hooks are executed afters.matchStep(step)
. Because of that, any changes in theStep
won't be applied and taken into consideration inmatchStep()
https://github.com/cucumber/godog/blob/afaebf26c1bc38e160c48cf921e65e52307fa3bd/suite.go#L137-L142
This behavior is newly introduced in
v0.12.0
, in the previous version, the hooks are executed beforematchStep()
https://github.com/cucumber/godog/blob/2b426f89696bd14865efdce2f001b75be9ae9efb/suite.go#L52-L59
I see this (breaking) change is unnecessary and would like to bring back the old behavior.
I don't know if we should consider this as a bug or not.
Motivation & context
I have some logic to dynamically replace the value in the step using
beforeStep
hook. For a (stupid) exampleAnd the step handler will be like
In
v0.12.0
,v
is still$FOOBAR
because the change is too late,matchStep
is executed with the original step definitionType of change
Note to other contributors
n/a
Update required of cucumber.io/docs
n/a
Checklist: