Skip to content

refactor(common): deprecate isPlatform functions #50055

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
Closed
JeanMeche wants to merge 1 commit into from
Closed

refactor(common): deprecate isPlatform functions #50055

JeanMeche wants to merge 1 commit into from

Conversation

@JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Apr 28, 2023 •
edited
Loading

The Web worker platform was removed in v11.
isPlatformWorkerApp & isPlatformWorkerUi have no purpose since, lets deprecate them!

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • No.

@JeanMeche JeanMeche force-pushed the chore/remove-platform-func branch 2 times, most recently from 6658102 to a28277e Compare April 28, 2023 00:47
@JeanMeche JeanMeche marked this pull request as ready for review April 28, 2023 00:50
@pullapprove pullapprove bot requested a review from atscott April 28, 2023 00:51
@pkozlowski-opensource pkozlowski-opensource added action: review The PR is still awaiting reviews from at least one requested reviewer area: common Issues related to APIs in the @angular/common package labels Apr 28, 2023
@ngbot ngbot bot modified the milestone: Backlog Apr 28, 2023
alan-agius4
alan-agius4 approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@alan-agius4 alan-agius4 added the target: rc This PR is targeted for the next release-candidate label Apr 28, 2023
atscott
atscott requested changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

Actually, these functions are public API. If they're exported, they can't be removed with deprecation and then waiting 2 major versions, right?

@JeanMeche
Copy link
Member Author

JeanMeche commented Apr 28, 2023 •
edited
Loading

Yeah you're right.
But they don't have any purpose since v11. So I was really wondering about their usage and if the 2 version depreciation period was necessary !

@atscott
Copy link
Contributor

atscott commented Apr 28, 2023

So I was really wondering about their usage and if the 2 version depreciation period was necessary !

Unless they were officially already deprecated in v11, yes

@JeanMeche JeanMeche force-pushed the chore/remove-platform-func branch from a28277e to e1999a5 Compare April 28, 2023 16:17
@JeanMeche
Copy link
Member Author

JeanMeche commented Apr 28, 2023

Let's deprecate them then !

@JeanMeche JeanMeche changed the title refactor(common): remove unused isPlatform functions refactor(common): deprecate isPlatform functions Apr 28, 2023
@pullapprove pullapprove bot requested a review from alxhub April 28, 2023 16:21
pkozlowski-opensource
pkozlowski-opensource approved these changes Apr 28, 2023
View reviewed changes
Copy link
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from atscott April 28, 2023 16:23
@JeanMeche
refactor(common): Deprecate useless isPlatform functions
36a3a0c 
The Web worker platform was removed in v11.
`isPlatformWorkerApp` & `isPlatformWorkerUi` have no purpose since.
@JeanMeche JeanMeche force-pushed the chore/remove-platform-func branch from e1999a5 to 36a3a0c Compare April 28, 2023 16:39
AndrewKushnir
AndrewKushnir approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@AndrewKushnir AndrewKushnir removed the request for review from atscott April 28, 2023 16:59
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 28, 2023
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Apr 28, 2023

Presubmit.

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Apr 28, 2023
@alxhub
Copy link
Member

alxhub commented May 1, 2023

This PR was merged into the repository by commit f49c6d0.

@alxhub alxhub closed this in f49c6d0 May 1, 2023
alxhub pushed a commit that referenced this pull request May 1, 2023
@JeanMeche @alxhub
refactor(common): Deprecate useless isPlatform functions (#50055)
5711bab 
The Web worker platform was removed in v11.
`isPlatformWorkerApp` & `isPlatformWorkerUi` have no purpose since.

PR Close #50055
@JeanMeche JeanMeche deleted the chore/remove-platform-func branch May 2, 2023 12:09
sr5434 pushed a commit to sr5434/angular that referenced this pull request May 3, 2023
@JeanMeche @sr5434
refactor(common): Deprecate useless isPlatform functions (angular#50055)
ed642b6 
The Web worker platform was removed in v11.
`isPlatformWorkerApp` & `isPlatformWorkerUi` have no purpose since.

PR Close angular#50055
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Jun 2, 2023

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@atscott atscott atscott approved these changes

@pkozlowski-opensource pkozlowski-opensource pkozlowski-opensource approved these changes

@alan-agius4 alan-agius4 alan-agius4 approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package target: rc This PR is targeted for the next release-candidate

Projects

None yet

Milestone

v16-final

Development

Successfully merging this pull request may close these issues.

6 participants

@JeanMeche @atscott @AndrewKushnir @alxhub @pkozlowski-opensource @alan-agius4