-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enable forward compatibility tests for 3.1.x #14633
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
Conversation
*/ | ||
final val ExperimentalVersion: Int = 1 | ||
final val ExperimentalVersion: Int = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we change this then the nightly will have a stable release version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like discussion on #14306 seems to agree with doing this however
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the point. That should be OK as until we release 3.1.3-RC1 and start working on 3.2.0-RC1-SNAPSHOT on the main branch we're forced to stick to 28.1-0 as TASTy format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a workaround #14306 rather than a fix. How would that solve the exact same problem for 3.2 when and later versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to indicate that all nightly and snapshots will be tagged as stable. Can't we do something more fine-grained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we will have to set this to 1
and it will brack the tests.
@nicolasstucki any remarks or can we get this merged? |
I'm still concerned about making the experimental field stable - for example we recently (accidentally) merged #14610 which bumped the minor version to 3.2.x - and several nightly releases came afterwards. If this PR was merged before then we would have released a nightly in the 3.2.x series with the same tasty version as the stable 3.1.0 |
Of course there's always a chance we do something wrong by accident. We could also have a bug which caused producing TASTy files uncompliant with an older format when |
*/ | ||
final val ExperimentalVersion: Int = 1 | ||
final val ExperimentalVersion: Int = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a workaround #14306 rather than a fix. How would that solve the exact same problem for 3.2 when and later versions?
*/ | ||
final val ExperimentalVersion: Int = 1 | ||
final val ExperimentalVersion: Int = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to indicate that all nightly and snapshots will be tagged as stable. Can't we do something more fine-grained?
* When the format gets fixed for a minor version of the language, | ||
* e.g. at the point of the release of Scala 3.2.0-RC1, | ||
* the experimental version number should be changed to 0 and after that | ||
* the same non-experimental verion of TASTy should be used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* the same non-experimental verion of TASTy should be used | |
* the same non-experimental version of TASTy should be used |
*/ | ||
final val ExperimentalVersion: Int = 1 | ||
final val ExperimentalVersion: Int = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we will have to set this to 1
and it will brack the tests.
Is this PR aligned with the formatting stated in #14306 (comment)? |
@prolativ is this something you plan on returning to? |
Most probably not. This PR was supposed to (1) is no more a problem as the idea of introducing @ckipp01 have you found any new arguments in favour of introducing this change? |
No I haven't, I just am doing some cleanup of old PRs. If I'm understanding you correctly, this can probably be closed then? |
@ckipp01 it might mean the ticket should be closed as well. You're like the guy on Monty Python during the Plague calling, "Bring out yer dead!" |
Fixes #14306