Skip to content

fix(core): Re-assign error codes to be within core bounds (<1000) #53455

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

fix(core): Re-assign error codes to be within core bounds (<1000) #53455

danieljancar wants to merge 1 commit into from

Conversation

@danieljancar
Copy link
Contributor

@danieljancar danieljancar commented Dec 8, 2023 •
edited
Loading

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?

Two error codes in the angular/core package incorrectly use error codes 1000 and 1001, which should be used for the reserved range in the angular/forms package.

Error code ranges per package:
 - core (this package): 100-999
 - forms: 1000-1999
 - common: 2000-2999
 - animations: 3000-3999
 - router: 4000-4999
 - platform-browser: 5000-5500

Issue Number: #53433

What is the new behavior?

This PR reassigns the error codes for RUNTIME_DEPS_INVALID_IMPORTED_TYPE and RUNTIME_DEPS_ORPHAN_COMPONENT from 1000 and 1001 to 950 and 951, to align with the reserved range for the core package.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Close #53433

@danieljancar danieljancar changed the title fix(core): reassign error codes for runtime dependencies Reassign error codes for runtime dependencies Dec 8, 2023
JeanMeche
JeanMeche reviewed Dec 8, 2023
View reviewed changes
@JeanMeche
Copy link
Member

JeanMeche commented Dec 8, 2023 •
edited
Loading

Hi, thanks for helping us with that change !

We also need to update our api golden files. Can you run yarn bazel run //packages/core:core_errors.accept and amend your commit ?

@danieljancar danieljancar changed the title Reassign error codes for runtime dependencies Reassign error codes for runtime dependencies #53433 Dec 9, 2023
@danieljancar danieljancar changed the title Reassign error codes for runtime dependencies #53433 Reassign error codes for runtime dependencies Dec 9, 2023
@JeanMeche
Copy link
Member

JeanMeche commented Dec 9, 2023

Could you please squash your commits into one and make sure the remaining one has a refactor(core): .... commit message.

Thx for the help !

@danieljancar danieljancar force-pushed the 53433-fix-error-code-beyond-expected-range branch from 2c641ed to 1b22b4d Compare December 10, 2023 17:03
@danieljancar
Copy link
Contributor Author

danieljancar commented Dec 11, 2023

Hey @JeanMeche,
I squashed the commits, should be ready now. 🚀

@JeanMeche JeanMeche added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime labels Dec 11, 2023
@ngbot ngbot bot modified the milestone: Backlog Dec 11, 2023
@JeanMeche
Copy link
Member

JeanMeche commented Jan 26, 2024

Hey, can you drop that merge commit ? We'd like the change to be a single commit !

@danieljancar danieljancar force-pushed the 53433-fix-error-code-beyond-expected-range branch from 7427d28 to 45b5abb Compare January 26, 2024 07:17
@danieljancar
Copy link
Contributor Author

danieljancar commented Jan 26, 2024

Hey, yeah is squashed. 😉

@JeanMeche
Copy link
Member

JeanMeche commented Jan 26, 2024

It looks like there is a conflict now, care to have a look at it ?

@danieljancar danieljancar force-pushed the 53433-fix-error-code-beyond-expected-range branch 2 times, most recently from 58e4b17 to 0131a73 Compare January 26, 2024 11:16
@pullapprove pullapprove bot requested a review from alan-agius4 January 26, 2024 11:17
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jan 26, 2024
@angular-robot angular-robot bot added the area: docs Related to the documentation label Jan 26, 2024
@pullapprove pullapprove bot requested a review from AndrewKushnir April 26, 2024 22:25
@AndrewKushnir AndrewKushnir removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 26, 2024
AndrewKushnir
AndrewKushnir approved these changes Apr 26, 2024
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

@pullapprove pullapprove bot requested a review from atscott April 26, 2024 22:26
@pullapprove pullapprove bot removed the request for review from atscott October 2, 2024 14:44
@danieljancar @JeanMeche
fix(core): Re-assign error codes to be within core bounds (<1000)
ae80d23 
`RUNTIME_DEPS_INVALID_IMPORTED_TYPE` is now 980
`RUNTIME_DEPS_ORPHAN_COMPONENT` is now 981
@JeanMeche JeanMeche force-pushed the 53433-fix-error-code-beyond-expected-range branch from 61d8146 to ae80d23 Compare October 2, 2024 15:02
@pullapprove pullapprove bot requested a review from thePunderWoman October 2, 2024 15:02
@angular-robot angular-robot bot added area: core Issues related to the framework runtime and removed area: core Issues related to the framework runtime labels Oct 2, 2024
@ngbot ngbot bot modified the milestones: v19 Feature Freeze Candidates, Backlog Oct 2, 2024
AndrewKushnir
AndrewKushnir approved these changes Oct 2, 2024
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 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 Oct 2, 2024
thePunderWoman
thePunderWoman approved these changes Oct 7, 2024
View reviewed changes
Copy link
Contributor

@thePunderWoman thePunderWoman 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

@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 7, 2024
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Oct 7, 2024

This PR was merged into the repository by commit 656b5d3.

The changes were merged into the following branches: main

@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Nov 7, 2024

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 Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JeanMeche JeanMeche JeanMeche left review comments

@thePunderWoman thePunderWoman thePunderWoman approved these changes

@alxhub alxhub alxhub approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

@alxhub alxhub

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

error code beyond expected range

6 participants

@danieljancar @JeanMeche @AndrewKushnir @alxhub @thePunderWoman @alan-agius4