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.

Sign up for GitHub

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

Jump to bottom

Feature/strict mode #211

Closed
wants to merge 3 commits into from
Closed

Feature/strict mode #211

wants to merge 3 commits into from

Conversation

elwynelwyn
Copy link
Contributor

This is a duplicate of lchenays add strict mode pull request, but with fixed/added tests for the new strict flag.
I also modified the --help message for the new flag to be inline with Ruby cucumber.

@samccone samccone mentioned this pull request Aug 4, 2014
Closed
@elwynelwyn
Copy link
Contributor Author

@samccone Thanks for the video. I've rebased all the things - squashed the branch down into two commits, which includes reverted release/cucumber.js back to how it is in master.

jbpros
jbpros reviewed Aug 8, 2014
View reviewed changes
lib/cucumber/cli.js
@@ -65,11 +65,14 @@ var Cli = function(argv) {
summary : prints a summary only, after all\n\
scenarios were executed\n\
\n\
-s, --strict Fail if there are any undefined or pending steps.\n\
Copy link
Member

Choose a reason for hiding this comment

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

The shorthand version of the flag is -S in cucumber-ruby. -s is for --no-source. I wish it was the other way around but I think we should not confuse people and stick to ruby's. Can you change this?

@elwynelwyn
Add strict mode
b252c90 
Pulling in strict implementation from lchenay. Modified the --help
message for strict to be inline with Ruby cucumber.
@elwynelwyn
Tests for strict mode
07031d9 
Fixed tests failing due to addition of new flag. Added tests for new
functionality in ast_tree_walker.
elwynelwyn
elwynelwyn reviewed Aug 10, 2014
View reviewed changes
lib/cucumber/cli.js
@@ -65,11 +65,14 @@ var Cli = function(argv) {
summary : prints a summary only, after all\n\
scenarios were executed\n\
\n\
-S, --strict Fail if there are any undefined or pending steps.\n\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbpros - I've updated the shorthand flag to be inline with the ruby implementation, -S

@samccone samccone mentioned this pull request Aug 12, 2014
Closed
@jbpros
Copy link
Member

jbpros commented Sep 5, 2014

Thanks @elwynelwyn, this is very nicely done.

@jbpros
Copy link
Member

jbpros commented Sep 5, 2014

@elwynelwyn could you rebase this onto master and add a couple of scenarios to test this e2e please?

@elwynelwyn
Copy link
Contributor Author

@jbpros I've added e2e scenarios, and had a go at rebasing to master - not sure if I did it right though?
I essentially did:

> git checkout master
Switched to branch 'master'
> git rebase feature/strict_mode
First, rewinding head to replay your work on top of it...
Fast-forwarded master to feature/strict_mode.
> git push

Let me know if you'd like me to do it differently!

@jeduden jeduden mentioned this pull request Oct 5, 2014
Closed
@jbpros jbpros closed this in 4bfc122 Dec 21, 2014
@jbpros
Copy link
Member

jbpros commented Dec 21, 2014

Thanks @elwynelwyn!

@rsshilli rsshilli mentioned this pull request Feb 2, 2018
Closed
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@elwynelwyn @jbpros @aslakhellesoy @samccone