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 public to the @compat macro #805

Merged
merged 6 commits into from
Sep 23, 2023
Merged

Add public to the @compat macro #805

merged 6 commits into from
Sep 23, 2023

Conversation

LilithHafner
Copy link
Member

The documentation included in this PR should make the PR itself self-explanatory. If it's not self-explanatory, then it's not acceptably documented.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #805 (d74c2f3) into master (c59d116) will increase coverage by 0.41%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #805      +/-   ##
==========================================
+ Coverage   92.72%   93.13%   +0.41%     
==========================================
  Files           2        2              
  Lines         316      335      +19     
==========================================
+ Hits          293      312      +19     
  Misses         23       23              
Files Changed Coverage Δ
src/Compat.jl 93.22% <ø> (ø)
src/compatmacro.jl 92.50% <100.00%> (+6.78%) ⬆️

src/Compat.jl Outdated Show resolved Hide resolved
@fredrikekre
Copy link
Member

Can I propose @compat public foo, bar?

@LilithHafner
Copy link
Member Author

Is @compat already used or would this be the only non-deprecated use of the @compat macro?

@fredrikekre
Copy link
Member

It used to be used for all these kind of things in the past, and it is still there:

macro compat(ex)

@LilithHafner
Copy link
Member Author

IIUC @compat was used for syntax changes in the pre-1.0 erra, and was rendered a no-op when Compat.jl dropped support for pre-1.0 code? If that history is correct, then @compat public foo, bar seems appropriate.

@fredrikekre
Copy link
Member

But that was because Compat.jl dropped support for pre Julia 1.0, no? Then there was no point in having all the old syntax transformations.

@LilithHafner
Copy link
Member Author

@fredrikekre, I think we are in agreement. I also prefer your proposed syntax. We should probably wait at least a day or so to let other folks chime in, though.

@LilithHafner LilithHafner changed the title Add @public macro Add public to the @compat macro Sep 15, 2023
@mcabbott
Copy link
Contributor

This does not seem to allow macros:

julia> @compat public fun, @mac
ERROR: LoadError: ArgumentError: cannot mark `(fun, #= REPL[15]:1 =# @mac())` as public. Try `@compat public foo, bar`.

@LilithHafner LilithHafner merged commit 90538e2 into master Sep 23, 2023
20 checks passed
@LilithHafner LilithHafner deleted the lh/public branch September 23, 2023 16:17
@LilithHafner
Copy link
Member Author

Should this be backported to 3.x?

@lgoettgens
Copy link

Should this be backported to 3.x?

I would be in favor of this. There are many packages out there that support julia versions older than 1.6 and are thus unable to depend on Compat 4.
Personally, I would really like to use @public via a Compat-compatible way in Aqua.

LilithHafner added a commit that referenced this pull request Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants