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.

Sign up for GitHub

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

Jump to bottom

chore: add deprecated app.runningUnderRosettaTranslation to breaking-changes.md #39897

Merged
merged 1 commit into from Sep 26, 2023
Merged

chore: add deprecated app.runningUnderRosettaTranslation to breaking-changes.md #39897

merged 1 commit into from Sep 26, 2023

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Sep 18, 2023 •
edited

Description of Change

It has been deprecated since #29168

Checklist

Release Notes

Notes: The app.runningUnderRosettaTranslation property has been deprecated.

@miniak miniak requested a review from a team as a code owner September 18, 2023 17:32
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 18, 2023
@miniak miniak added semver/major incompatible API changes target/27-x-y PR should also be added to the "27-x-y" branch. labels Sep 18, 2023
@miniak miniak self-assigned this Sep 18, 2023
@miniak miniak changed the title chore: add deprecated app.runningUnderRosettaTranslation to breaking-changes.md chore: add deprecated app.runningUnderRosettaTranslation to breaking-changes.md Sep 18, 2023
erickzhao
erickzhao reviewed Sep 18, 2023
View reviewed changes
Copy link
Member

@erickzhao erickzhao left a comment

Choose a reason for hiding this comment

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

Since it's been deprecated in code since Electron 15, do we want to add it to the breaking changes retroactively for that version?

@miniak
Copy link
Contributor Author

miniak commented Sep 18, 2023

Since it's been deprecated in code since Electron 15, do we want to add it to the breaking changes retroactively for that version?

can I do that?

@RaisinTen
Copy link
Contributor

I had updated the breaking changes doc for older versions in the past (#34787 for example), so I think it should be doable.

@erickzhao
Copy link
Member

If it's just an omission in the doc I don't think there's anything code-wise stopping us from adding it in a more accurate spot.

@miniak miniak added target/25-x-y PR should also be added to the "25-x-y" branch. target/26-x-y PR should also be added to the "26-x-y" branch. labels Sep 19, 2023
@miniak miniak added semver/none and removed semver/major incompatible API changes api-review/requested 🗳 labels Sep 19, 2023
@miniak miniak force-pushed the miniak/deprecate-rosetta-api branch 2 times, most recently from b558351 to 9ddb772 Compare September 19, 2023 13:29
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 19, 2023
@dsanders11
Copy link
Member

IMO we should avoid retroactively changing breaking-changes.md as that's unexpected, and works against the point of breaking-changes.md, which is to ensure app developers are made aware of breaking changes. Inserting it into that doc for an old unsupported version doesn't work toward that goal, IMO.

Darshan's past example seems fine to me because it updated breaking-changes.md to reflect the reality that removals occurred in those versions. In this case the reality is this API wasn't properly deprecated (since it didn't appear in breaking-changes.md) at the time, so this is now the version we're properly deprecating it in.

@codebytere
Copy link
Member

@dsanders11 how do you think this PR should move forward then?

@miniak
Copy link
Contributor Author

miniak commented Sep 21, 2023

@codebytere, @dsanders11 we can formally deprecate it in Electron 27 and remove it in Electron 28

@jkleinsc
Copy link
Contributor

@miniak @electron/wg-releases is discussing what to do with this. We should have an update today.

@miniak miniak mentioned this pull request Sep 22, 2023
Merged
5 tasks
@miniak
Copy link
Contributor Author

miniak commented Sep 25, 2023

@jkleinsc I saw the discussion on Slack and it looked like we should proceed with the PR as is.

@Urook
Copy link

Urook commented Sep 26, 2023

Maybe an additional note should be added to the upcoming "removed" section? One that points out it was deprecated recently, the version it was deprecated from, etc.?

@jkleinsc jkleinsc merged commit fa215f1 into main Sep 26, 2023
16 checks passed
@jkleinsc jkleinsc deleted the miniak/deprecate-rosetta-api branch September 26, 2023 17:42
@release-clerk
Copy link

release-clerk bot commented Sep 26, 2023

Release Notes Persisted

The app.runningUnderRosettaTranslation property has been deprecated.

This was referenced Sep 26, 2023
Merged
Merged
Merged
@trop
Copy link
Contributor

trop bot commented Sep 26, 2023

I have automatically backported this PR to "25-x-y", please check out #39982

@trop trop bot added in-flight/25-x-y and removed target/25-x-y PR should also be added to the "25-x-y" branch. labels Sep 26, 2023
@trop
Copy link
Contributor

trop bot commented Sep 26, 2023

I have automatically backported this PR to "26-x-y", please check out #39983

@trop
Copy link
Contributor

trop bot commented Sep 26, 2023

I have automatically backported this PR to "27-x-y", please check out #39984

@trop trop bot added in-flight/26-x-y in-flight/27-x-y merged/27-x-y PR was merged to the "27-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. merged/25-x-y PR was merged to the "25-x-y" branch. and removed target/26-x-y PR should also be added to the "26-x-y" branch. target/27-x-y PR should also be added to the "27-x-y" branch. in-flight/27-x-y in-flight/26-x-y in-flight/25-x-y labels Sep 26, 2023
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
@miniak
chore: add deprecated app.runningUnderRosettaTranslation to breakin…
22c72af 
…g-changes.md (electron#39897)

chore: add deprecated app.runningUnderRosettaTranslation to breaking-changes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erickzhao erickzhao erickzhao left review comments

@codebytere codebytere codebytere approved these changes

@dsanders11 dsanders11 dsanders11 approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees

@miniak miniak

Labels
merged/25-x-y PR was merged to the "25-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. merged/27-x-y PR was merged to the "27-x-y" branch. semver/none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@miniak @RaisinTen @erickzhao @dsanders11 @codebytere @jkleinsc @Urook