Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Don't deprecate splat #48038

Merged
merged 1 commit into from
Jan 13, 2023
Merged

Don't deprecate splat #48038

merged 1 commit into from
Jan 13, 2023

Conversation

knuesel
Copy link
Member

@knuesel knuesel commented Dec 29, 2022 •
edited
Loading

As discussed in #47714, this keeps the functionality introduced in #42717 but using the existing splat API, exporting splat instead of Splat.

This targets the release-1.9 branch since there are modifications to the 1.9 version of NEWS.md.

@LilithHafner LilithHafner added the triage This should be discussed on a triage call label Dec 29, 2022
@LilithHafner
Copy link
Member

The standard way we do backports (changes that should be made to a version which has already had its feature freeze) is merging a PR to master and tagging it with the appropriate backport label (in this case backport 1.9). That way the change is present in master and all future releases as well. Someone (e.g. @KristofferC for the 1.9 release) will merge the changes in this PR into the 1.9 release branch after it merges into master.

This has been somewhat controversial, so I'm tagging this and #47714 (the issue with most of the discussion on the topic) with the triage label so folks will discuss it on a video call. You are welcome to join! See the #triage slack channel for logistical info.

@LilithHafner LilithHafner changed the base branch from release-1.9 to master December 29, 2022 14:29
@LilithHafner LilithHafner changed the base branch from master to release-1.9 December 29, 2022 14:29
Seelengrab
Seelengrab reviewed Dec 29, 2022
View reviewed changes
base/operators.jl Outdated Show resolved Hide resolved
@knuesel
Copy link
Member Author

knuesel commented Dec 29, 2022

@LilithHafner OK sorry and thanks for the correction. Should I split this into a main PR on master, and a small one on release-1.9 for the change to NEWS.md?

@Seelengrab
Copy link
Contributor

The backports will have their news aggregated with their version bump, if I recall correctly, so just a PR targeting master should be enough.

@knuesel knuesel force-pushed the keep-lowercase-splat branch from 7623da0 to 6259887 Compare December 30, 2022 13:08
@knuesel
Don't deprecate splat
ee64566 
Keep the splat function and mention in the documentation that it's the
recommended way of constructing a Base.Splat object.
@knuesel knuesel force-pushed the keep-lowercase-splat branch from 6259887 to ee64566 Compare December 30, 2022 13:24
@knuesel knuesel changed the base branch from release-1.9 to master December 30, 2022 13:25
@knuesel
Copy link
Member Author

knuesel commented Dec 30, 2022

I've updated the PR to target master. I dropped the change to NEWS.md, I guess it will have to go in another PR on backports-release-1.9 if the present PR is accepted (the change includes removing a line from the 1.9 version of NEWS.md, which cannot be done when targeting master).

@Seelengrab
Copy link
Contributor

Seelengrab commented Dec 30, 2022 •
edited
Loading

What I meant was that there's no need to take special consideration of the backports-release-1.9 branch - just do all changes that ought to land in master here, including to NEWS.md, and if this PR is backported the changes will make it to the backport branch as well.

Ah, you're referring to master already having the NEWS.md for 1.10 - my bad, sorry for the noise!

@knuesel
Copy link
Member Author

knuesel commented Dec 30, 2022

Yes exactly (I should have said that more clearly).

@LilithHafner LilithHafner added backport 1.9 Change should be backported to release-1.9 and removed triage This should be discussed on a triage call labels Jan 5, 2023
@LilithHafner
Copy link
Member

Triage approves

@KristofferC KristofferC mentioned this pull request Jan 10, 2023
Merged
41 tasks
@KristofferC KristofferC merged commit 670190c into JuliaLang:master Jan 13, 2023
KristofferC pushed a commit that referenced this pull request Jan 13, 2023
@knuesel @KristofferC
Don't deprecate splat (#48038)
68f4230 
Keep the splat function and mention in the documentation that it's the
recommended way of constructing a Base.Splat object.

(cherry picked from commit 670190c)
@mcabbott mcabbott mentioned this pull request Jan 13, 2023
Merged
@knuesel knuesel mentioned this pull request Jan 16, 2023
Closed
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Jan 17, 2023
@martinholters martinholters mentioned this pull request Jan 18, 2023
Merged
@knuesel knuesel mentioned this pull request Jan 19, 2023
Merged
@inkydragon inkydragon mentioned this pull request Mar 10, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Seelengrab Seelengrab Seelengrab left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@knuesel @LilithHafner @Seelengrab @KristofferC