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

Add defaults values to context dependent destinations #29

Merged
merged 2 commits into from Jan 16, 2018

Conversation

danpalmer
Copy link
Contributor

No description provided.

Copy link
Contributor

@PeterJCLaw PeterJCLaw left a comment

Choose a reason for hiding this comment

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

Just to check my understanding -- the default is required for nexts of type context?

@@ -78,18 +78,19 @@ class ContextNextStates(NamedTuple):
"""Defined a choice based on a path in the given `label_context`."""
path: str
destinations: Iterable[ContextNextStatesOption]
default: str # noqa: E704
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, since I can't find this despite searching, what error is E704? (I'd really like to see flake8 support pylint-style error names, but alas)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a comment, but it's basically a bug somewhere in flake8 or pycodestyle. I tried to create a failing test case for it in pycodestyle, but wasn't able to reproduce the failure (I'm not sure I understand their test case DSL format) and also that DSL doesn't seem to support differences between Python 2 and 3, and this is caused by the type annotation I believe.

My best guess is that this matches a regex that looks roughly like ^\s+def.+:.$

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0009%) to 99.164% when pulling 0860b54 on default-context-next-state into 725f6f1 on master.

@danpalmer danpalmer merged commit 658ed43 into master Jan 16, 2018
@danpalmer danpalmer deleted the default-context-next-state branch January 16, 2018 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants