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

Announce new function Base.isrelocatable in NEWS for v1.11 #54444

Closed

Conversation

fatteneder
Copy link
Contributor

No description provided.

@KristofferC
Copy link
Sponsor Member

I think this function promises more than what it delivers. As far as I understand it just checks that the source files are @depot-relocatable but full relocatability requires more (#54430) and is hard to even programmatically verify.

Also, this is an internal function right so it doesn't really need to be in NEWS at all?

@fatteneder
Copy link
Contributor Author

fatteneder commented May 13, 2024

I think this function promises more than what it delivers. As far as I understand it just checks that the source files are @depot-relocatable but full relocatability requires more (#54430) and is hard to even programmatically verify.

I agree.
However, I think Base.isrelocatable delivers what it promises if pkg authors adhere to Pkg.jl's best practices that say that a pkg's code shall not assume that its sources are placed in a stable location.
Should I update its docstring to make this more clear?
Or do you have a suggestion for a more fitting name?

Also, this is an internal function right so it doesn't really need to be in NEWS at all?

I thought that function should accompany Base.compilecache. However, I just noticed that that one is also internal.
Feel free to close if you think they should be kept internal, otherwise I can make a PR to make them public.

@KristofferC
Copy link
Sponsor Member

Feel free to close if you think they should be kept internal, otherwise I can make a PR to make them public.

At least for now, I think it is fine the way it is.

@fatteneder fatteneder deleted the fa/news-isrelocatable branch May 13, 2024 10:32
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

2 participants