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

Fixes #335 #336

Merged
merged 1 commit into from Mar 2, 2018

Conversation

Projects
None yet
3 participants
@link89
Contributor

link89 commented Feb 23, 2018

@jenisys

Improves implementation and overcomes mutability problem of the current implementation (what is probably meant w/ thread-safety).

@brasmusson

After contemplating on the "variable or ctor" construct, I think it falls within the "Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value." recommendation of the PEP8 programming recommendations, so I think that it should rather be (in __init__):

self.ast_builder = ast_builder if ast_builer is not None else AstBuilder()

and (in parse):

if token matcher is None:
    token_matcher = TokenMatcher()

Please change the pull request description to Fixes #335, Closes #335 etc, to take advantage of the automatic closing of issues when this pull request is merged.

@link89

This comment has been minimized.

Contributor

link89 commented Mar 1, 2018

It's kind of you to point this out

@link89 link89 changed the title from gherkin: python thread safe to Fixes #335 Mar 1, 2018

@link89 link89 merged commit 77b94f8 into cucumber:master Mar 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment