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

Added 3.4.2 support in a fairly formulaic fashion #1503

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

Ichoran
Copy link
Contributor

@Ichoran Ichoran commented Jun 7, 2024

Added 3.4.2 to targets in a fairly straightforward, formulaic fashion. 3.4.0 and 3.4.1, for good reason, fail to parse $ as a class file, but a temporary ad-hoc workaround was added in 3.4.2, so this needn't block 3.4 support for now. Reliance on object $ still needs to be fixed in Ammonite.

Several internals changes required fixes also; I used the most obvious solution (e.g. if Y extends X and Y is now private, use X instead), but someone who knows more might want to read it over.

Tests pass on my machine for 3.3 and 3.4.2 (under Java 21).

If accepted, this patch would supersede #1394

…versions

Fixed internals changes and tests that specify output.
@Ichoran
Copy link
Contributor Author

Ichoran commented Jun 7, 2024

If it works, this should complete #1425

@lihaoyi
Copy link
Member

lihaoyi commented Jun 10, 2024

@Ichoran how much of the stuff in the 3.4.2+/ folder really needs to be duplicated? I wonder if it actually involves a widespread change to the relevant code, or whether we just need to duplicate a few helper functions and the rest of the code can remain shared for easier maintainability

@Ichoran
Copy link
Contributor Author

Ichoran commented Jun 10, 2024

@lihaoyi - Yeah, I wasn't entirely happy with the code duplication, but that's what was already done for 3.0 vs 3.3.2+ (i.e. if there is a change in a file, duplicate the file in sub-version-specific folders and remove it from scala-3), so I just continued the pattern. The changes are not extensive, and could be refactored out, but that should be done for 3.0 vs 3.3.2+ also if so.

I'm happy to take a look at that, but I'm not sure that should block 3.4 support given that it's already on the late side.

I can only plead, for this PR: I may have dug a hole, but it isn't a new hole; I'm just digging the existing one deeper.

@lihaoyi
Copy link
Member

lihaoyi commented Jun 10, 2024

sgtm

@lihaoyi lihaoyi merged commit 88291dd into com-lihaoyi:main Jun 10, 2024
38 checks passed
@lefou lefou added this to the 3.0.0-M3 milestone Jun 12, 2024
@lihaoyi
Copy link
Member

lihaoyi commented Jun 12, 2024

This is available as //github.com/com-lihaoyi/Ammonite/releases/download/3.0.0-M2/3.4-3.0.0-M2-9-88291dd8

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

3 participants