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

Fixes Issue 56 #58

Merged
merged 6 commits into from
Sep 23, 2016
Merged

Fixes Issue 56 #58

merged 6 commits into from
Sep 23, 2016

Conversation

thenewguy
Copy link

Fixes Issue 56

#56

jobtastic/task.py:152:1: W293 blank line contains whitespace

jobtastic/tests/test_broker_fallbacks.py:118:66: E231 missing whitespace
after ':'

jobtastic/tests/test_broker_fallbacks.py:124:66: E231 missing whitespace
after ':'

jobtastic/tests/test_broker_fallbacks.py:129:67: E231 missing whitespace
after ':'

jobtastic/tests/test_broker_fallbacks.py:135:67: E231 missing whitespace
after ':'

jobtastic/tests/test_broker_fallbacks.py:200:66: E231 missing whitespace
after ':'

jobtastic/tests/test_broker_fallbacks.py:209:67: E231 missing whitespace
after ':'
@@ -349,6 +349,16 @@ all of your arguments must be keyword arguments.
Note: This is a limitation of the current `significant_kwargs` implementation,
and totally fixable if someone wants to submit a pull request.

### async_or_eager
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Could we list these two below the delay_or versions? My impression is that most folks will continue to use the simpler signature of delay_or.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think listing the two async_ versions after the delay_ versions still makes sense.

Copy link
Contributor

@winhamwr winhamwr left a comment

Choose a reason for hiding this comment

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

WIth just that documentation re-order, I think this is ready to merge.

@thenewguy
Copy link
Author

Fixed. I thought I had already addressed your comments but we were waiting on me =P

@winhamwr
Copy link
Contributor

Do you think it's worth adding a test on async_or_FOO that actually sets one of the advanced task options that we're trying to get access to and verifies that it was used?

@winhamwr
Copy link
Contributor

Looking through the documentation of result, I don't see anything that would change as a result of something we passed in to apply_async, so this might be tough to test. Merging as-is.

@winhamwr winhamwr merged commit 8498ded into PolicyStat:master Sep 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants