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

Make experimental packages more self-contained #3409

Merged
merged 10 commits into from
Mar 15, 2024

Conversation

lgoettgens
Copy link
Member

Resolves the first bullet point in the list in #3408.

With this PR, removing an experimental project is now as easy as deleting the corresponding folder in experimental/ (and maybe removing it from oldexppkgs or orderedpkgs in experimental/Experimental.jl), in particular using Oscar and Aqua.test_undefined_exports(Oscar) succeed. The exemption is experimental/Rings/ which, currently, needs to be available for Oscar to load.
Tests in general, will not work yet, see #3408 for a roadmap on how to achieve that.

This PR does not change any current behaviour/exports/deprecations/etc., it just moves them to a more suited places.

@lgoettgens lgoettgens added the backport 1.0.x Should be backported to the release 1.0 branch label Feb 21, 2024
@benlorenz
Copy link
Member

@lgoettgens I am not really sure why this should be in 1.0, it is mostly meant to help keep the code-bases separate but there will not be much further development on the release branch. Moving lots of stuff around now seems not helpful for the release.

@lgoettgens
Copy link
Member Author

Yes, you are right. I think my intention was to reduce the number of potential conflicts occurring while backporting

@lgoettgens lgoettgens removed the backport 1.0.x Should be backported to the release 1.0 branch label Feb 22, 2024
@benlorenz
Copy link
Member

Maybe it would make sense to wait with merging this PR until we are done with most of the backporting, to avoid any conflicts, both for master and for the backports.

@lgoettgens
Copy link
Member Author

Yeah, waiting a few more days until all of the feature backports are done is more than fine. The minor doc changes later shouldn't become an issue with conflicts

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Looks good to me

@benlorenz benlorenz marked this pull request as draft February 22, 2024 19:03
@benlorenz
Copy link
Member

I changed it to draft so that we don't accidentally merge this right now, we can change it back some time next week I guess.

@fieker
Copy link
Contributor

fieker commented Feb 23, 2024

What is the motivatio to further tinker with the experimental stuff? As far as I can see: it works.
Is there any real, known, problem with the current situation?

@lgoettgens
Copy link
Member Author

What is the motivatio to further tinker with the experimental stuff? As far as I can see: it works. Is there any real, known, problem with the current situation?

Please see #3408 for the motivation. And let's keep the fundamental discussion there.

Copy link
Member

@lkastner lkastner 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 taking care of this.

@lgoettgens lgoettgens marked this pull request as ready for review March 13, 2024 11:13
@lgoettgens
Copy link
Member Author

Maybe it would make sense to wait with merging this PR until we are done with most of the backporting, to avoid any conflicts, both for master and for the backports.

Since 1.0 is now released and the bulk of backports is done, I think we could continue with this.

@lgoettgens lgoettgens added the experimental Only changes experimental parts of the code label Mar 13, 2024
@lgoettgens
Copy link
Member Author

Can we get this merged before any more conflicts come up?

@benlorenz benlorenz enabled auto-merge (squash) March 15, 2024 10:24
@benlorenz benlorenz merged commit 39a24f2 into oscar-system:master Mar 15, 2024
22 of 23 checks passed
@lgoettgens lgoettgens deleted the lg/old-experimental-include branch March 15, 2024 12:11
Syz-MS added a commit to Syz-MS/Oscar.jl that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Only changes experimental parts of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants