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

Trees: add interleaved param groups to Def/Macro for parameter clause interleaving #2929

Merged
merged 3 commits into from
Feb 7, 2023

Conversation

kitbellew
Copy link
Contributor

Also, add Dialect.allowParamClauseInterleaving. Based on #2820.

@kitbellew
Copy link
Contributor Author

kitbellew commented Oct 27, 2022

@Sporarum fyi

@kitbellew kitbellew changed the title Trees: add Interleaved{Def, Macro} for parameter clause interleaving Trees: add interleaved param groups to Def/Macro for parameter clause interleaving Dec 2, 2022
@kitbellew
Copy link
Contributor Author

@tgodzik ready to go.

@tgodzik
Copy link
Collaborator

tgodzik commented Dec 5, 2022

@tgodzik ready to go.

Looks great! I will take a look a bit later this week, but are we ok releasing a new version before this gets merged just in case? 4.7.0 most likely, no?

@kitbellew
Copy link
Contributor Author

@tgodzik ready to go.

Looks great! I will take a look a bit later this week, but are we ok releasing a new version before this gets merged just in case? 4.7.0 most likely, no?

if this is likely to be released, but we want to do it in next release, then i'd still like to add an intermediate change in this release. i will send it shortly.

@kitbellew
Copy link
Contributor Author

@tgodzik ready to go.

Looks great! I will take a look a bit later this week, but are we ok releasing a new version before this gets merged just in case? 4.7.0 most likely, no?

if this is likely to be released, but we want to do it in next release, then i'd still like to add an intermediate change in this release. i will send it shortly.

please take a look: #2987.

Copy link

@Sporarum Sporarum left a comment

Choose a reason for hiding this comment

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

I believe this implements faithfully what is in the SIP
(given both the implementation and the tests)

Really nice work, thank you !

@tgodzik
Copy link
Collaborator

tgodzik commented Jan 30, 2023

I think we should only merge this one after scala/scala3#14019 is finalized.

@Sporarum
Copy link

Sporarum commented Feb 3, 2023

scala/scala3#14019 has been merged !

But it will of course not be usable until the next release of Scala

Copy link
Collaborator

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

lampepfl/dotty#14019 has been merged !

But it will of course not be usable until the next release of Scala

It should be experimental in 3.3.x, right? We can merge it anyway in Scala313 dialect I think, should safe.

@kitbellew
Copy link
Contributor Author

kitbellew commented Feb 3, 2023

scala/scala3#14019 has been merged !
But it will of course not be usable until the next release of Scala

It should be experimental in 3.3.x, right? We can merge it anyway in Scala313 dialect I think, should safe.

meaning, instead of Scala3Future, add the flag to another dialect?

@tgodzik
Copy link
Collaborator

tgodzik commented Feb 3, 2023

meaning, instead of Scala3Future, add the flag to another dialect?

Yeah, I would make it into the default Scala3 dialect, unless it could cause issues?

So instead of replying I edited your answer 😵‍💫 Sorry about that! :|

The answer:
I would make it into the default Scala3 dialect, unless it could cause issues?

@Sporarum
Copy link

Sporarum commented Feb 6, 2023

It should be experimental in 3.3.x, right?

As soon as 3.3.1

@kitbellew
Copy link
Contributor Author

meaning, instead of Scala3Future, add the flag to another dialect?

Yeah, I would make it into the default Scala3 dialect, unless it could cause issues?

So instead of replying I edited your answer 😵‍💫 Sorry about that! :|

The answer: I would make it into the default Scala3 dialect, unless it could cause issues?

Done. Added to 3.3 (which is what scala3 points to), along with fewer braces.

@kitbellew kitbellew merged commit 8267ef7 into scalameta:main Feb 7, 2023
@kitbellew kitbellew deleted the 2820 branch February 7, 2023 02:47
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