-
Notifications
You must be signed in to change notification settings - Fork 56
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
Simplify JobContext #1212
Comments
Closed by #1213 |
(as seen in #1257 (comment), maybe the task was useful after all...) |
Rather than reintroducing a circular reference with the |
I think it was pretty useful to have the task there in the end. Is there really no simple way we can make it work for typing ? |
Sorry, I don't have a better suggestion. |
Then let's re-add the task to the JobContext and have a beta v3 release (it's the last issue left in v3 milestone). @onlyann, do you want to re-add it, or should I? |
Happy for you to add it @medihack. Looking back at the codebase, the JobContext has the Effectively, I think you can simply add a new property/method that returns:
|
Good point. Then, I don't see a need to do anything (maybe add a hint to the documentation on how to get the task) and keep the API simple. @ewjoachim Ok for you, too? |
Ok ! |
Cool. I will add a small hint about retrieving the task with its parameters to the documentation under the "Get more context for task execution" section and then close the issue. |
The
JobContext
class mixes fields that are intended to be used by consumers of this library while other fields are used internally.As part of
v3
, it is a good time to clean upJobContext
and only expose the fields that are intended for public use.There is also the opportunity to simplify typing by removing the
task
field.The text was updated successfully, but these errors were encountered: