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

Mark ScopedValue, with and @with as public #55095

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakobnissen
Copy link
Contributor

This is exported from the ScopedValues module into Base, but not exported from Base, and therefore not marked as public.

This is exported from the ScopedValues module into Base, but not exported
from Base, and therefore not marked as public.
@nsajko

This comment was marked as resolved.

@nsajko
Copy link
Contributor

nsajko commented Jul 10, 2024

BTW, I don't think you want to tag the Github user @with in the commit message.

@nsajko nsajko added the modules label Jul 10, 2024
@nsajko
Copy link
Contributor

nsajko commented Jul 10, 2024

Updates #51335

Follows up on #54574

@jakobnissen
Copy link
Contributor Author

It can't be marked as public inside ScopedValues because it is exported (this errors). It also makes no sense to export it from ScopedValues to Base, have it be public in ScopedValues but private in Base. If it was supposed to be private in Base, why is it exported to Base?

@CameronBieganek
Copy link
Contributor

CameronBieganek commented Jul 12, 2024

If it was supposed to be private in Base, why is it exported to Base?

@with is used in a couple places in Base, and also one place in the unit tests. (I mean in addition to the unit tests for the ScopedValues module.)

with and @with are extremely generic names, which I think is why some folks would like to keep those names contained in Base.ScopedValues. My preferred access pattern is either of the following:

using Base.ScopedValues

# or:

using Base.ScopedValues: ScopedValue, with

I would like to add the ScopedValues module to the Base export list (as is done for Iterators, Threads, and Sys), but it was pointed out that that could cause confusion if folks are also trying to use the ScopedValues.jl package for backwards compatibility. (See #54556.)

@LilithHafner
Copy link
Member

LilithHafner commented Aug 6, 2024

I don't think these names should be loaded into Base. I'd rather see Base.ScopedValues. IMO Base should load ScopedValues with import, which would make @CameronBieganek's access patterns the only viable patters (loading from Base would no longer be an option).

oscardssmith pushed a commit that referenced this pull request Aug 15, 2024
Stop loading `ScopedValues` with `using` so folks use
`ScopedValues.with` or `using ScopedValues` rather than `Base.with`.

Implements
#55095 (comment)

~Have to bump the StyledStrings stdlib to include
JuliaLang/StyledStrings.jl#80 Done

---------

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024
Stop loading `ScopedValues` with `using` so folks use
`ScopedValues.with` or `using ScopedValues` rather than `Base.with`.

Implements
JuliaLang#55095 (comment)

~Have to bump the StyledStrings stdlib to include
JuliaLang/StyledStrings.jl#80 Done

---------

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
KristofferC pushed a commit that referenced this pull request Aug 19, 2024
Stop loading `ScopedValues` with `using` so folks use
`ScopedValues.with` or `using ScopedValues` rather than `Base.with`.

Implements
#55095 (comment)

~Have to bump the StyledStrings stdlib to include
JuliaLang/StyledStrings.jl#80 Done

---------

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
(cherry picked from commit e1aefeb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants