Use ScopedValue to fix async issue
#112
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/deprecated.jl
Outdated
| @@ -12,3 +12,5 @@ function enable(; force=false) | |||
| depwarn("`$m.enable(; force=$force)` is deprecated, use `$m.activate()` instead.", :enable) | |||
| activate() | |||
| end | |||
|
|
|||
| @deprecate set_active_env(f, pe) with_active_env(f, pe) false | |||
There was a problem hiding this comment.
This isn't a proper deprecation as set_active_env would have it's changes persist beyond the scope of f. I think we should probably just make this a breaking change
There was a problem hiding this comment.
fair, though this is what the previous change did, modulo also renaming set_active_env.
That function isn't actually exported.
Since the normal way to do it is to use apply(pe) do.
More an internal implementation detail of apply ?
There was a problem hiding this comment.
I think I'm okay with stating that set_active_env and get_active_env are internal functions and don't require deprecations. Making this a breaking change would be the safest but I could be convinced to have this be non-breaking.
There was a problem hiding this comment.
If it breaks something can always revert, tag patch, and rerelease as breaking
There was a problem hiding this comment.
I will just delete them both and not deprecate
src/deprecated.jl
Outdated
| @@ -12,3 +12,5 @@ function enable(; force=false) | |||
| depwarn("`$m.enable(; force=$force)` is deprecated, use `$m.activate()` instead.", :enable) | |||
| activate() | |||
| end | |||
|
|
|||
| @deprecate set_active_env(f, pe) with_active_env(f, pe) false | |||
There was a problem hiding this comment.
I think I'm okay with stating that set_active_env and get_active_env are internal functions and don't require deprecations. Making this a breaking change would be the safest but I could be convinced to have this be non-breaking.
|
Going to kick start the CI workflow |
Co-authored-by: Curtis Vogt <[email protected]>
omus
changed the title
ScopedValue to fix async issue
|
The change here broke the test case on Julia 1.11+ which was testing for issue #108. I'll roll this change back temporarily and include it in a breaking release. |
* Use ContextVariablesX to fix async issue * Use ScopedValues rather than ContextVariables * Apply suggestions from code review Co-authored-by: Curtis Vogt <[email protected]> * Define missing PATCH_ENV * Refactor tasks testset into async scope * Import ScopedValue alongside other imports * Use VERSION check instead --------- Co-authored-by: Curtis Vogt <[email protected]>
* Use `ScopedValue` to fix async issue (#112) * Use ContextVariablesX to fix async issue * Use ScopedValues rather than ContextVariables * Define missing PATCH_ENV * Refactor tasks testset into async scope * Import ScopedValue alongside other imports * Use VERSION check instead --------- Co-authored-by: Curtis Vogt <[email protected]> * Test against Julia 1.11 * Update async-world-ages tests --------- Co-authored-by: Frames White <[email protected]>
In julia 1.11 we get a proper way to do what ContextVariablesX did with an hack.
This is a rebasing of #91 with that switch over
it should not run into the issues that required #93