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 getCurrentJob and getCurrentScopeJob functions #2245

Closed
wants to merge 4 commits into from

Conversation

recheej
Copy link
Contributor

@recheej recheej commented Sep 12, 2020

Resolves #2159 .

Adding different names for the suspend and CoroutineScope extension versions since there can be some confusion. Added docs and did api dump. Also replaced places in the code base (mostly tests) that accessed jobs with !!

@elizarov
Copy link
Contributor

@recheej Can you, please, elaborate what's your use-case for these functions? What kind of code needs them?

@recheej
Copy link
Contributor Author

recheej commented Sep 21, 2020

@elizarov I personally would use these functions for debugging coroutines code (see state of job, children, etc). But I could also see them being used for lower level coroutines work or for logging state of things.

In the end, I think the Job is such an important context element that it deserves its own function(s).

@qwwdfsad qwwdfsad self-requested a review September 24, 2020 09:58
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
We've discussed it internally and decided that it's not the addition we are ready to make. It's unclear who and how is going to use it, its naming is misaligned with existing memebrs (e.g. CoroutineScope.coroutineContext, but currentCoroutineContext()).

We'd like to introduce the only coroutineContext.job: Job? extension property for the sake of brevity where it is necessary. If you want to do this extra-mile and replace your PR with that, we'd be happy to help you with review and integration. If not, it's completely okay to tell us, then we are going to implement it ourselves

@recheej
Copy link
Contributor Author

recheej commented Sep 24, 2020

Sure I can do that change

@recheej recheej closed this Oct 17, 2020
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.

None yet

3 participants