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

fix(core): improve docs on afterRender hooks #56522

Closed

Conversation

cexbrayat
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

The AfterPhaseRender deprecation warning has a typo.

What is the new behavior?

This commit fixes a typo in the AfterRenderPhase deprecation warning and improves the documentation of the options parameter of the afterRender hooks (which are now all named options instead of being called opts in some functions and options in others).

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This commit fixes a typo in the `AfterRenderPhase` deprecation warning and improves the documentation of the options parameter of the afterRender hooks (which are now all named `options` instead of being called `opts` in some functions and `options` in others).
@pullapprove pullapprove bot requested a review from devversion June 19, 2024 17:46
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Jun 19, 2024
@ngbot ngbot bot added this to the Backlog milestone Jun 19, 2024
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.

@cexbrayat thanks for the improvement 👍

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

@pullapprove pullapprove bot requested a review from alxhub June 19, 2024 21:12
@AndrewKushnir AndrewKushnir added target: patch This PR is targeted for the next patch release action: presubmit The PR is in need of a google3 presubmit labels Jun 19, 2024
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note PullApprove: disable and removed action: presubmit The PR is in need of a google3 presubmit labels Jun 20, 2024
@AndrewKushnir
Copy link
Contributor

Caretaker notes:

  • presubmit is "green"
  • there is no real change to the public API (only argument rename), no need for extra reviews
  • this PR is ready for merge

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
Reviewed-for: fw-core

@AndrewKushnir AndrewKushnir added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 20, 2024
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 86bcfd3.

The changes were merged into the following branches: main

@AndrewKushnir
Copy link
Contributor

@cexbrayat there was a merge conflict with the 18.0.x branch, so this PR was merged into the main branch only. Could you please create a new PR and target the 18.0.x branch when you get a chance? Thank you.

@cexbrayat
Copy link
Member Author

@AndrewKushnir Here it is #56525 As the main part of the PR is based on changes introduced in the next release, the new PR is really small (and maybe not worth it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note PullApprove: disable target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants