Skip to content

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

Closed
danieljancar wants to merge 1 commit into
angular:mainfrom
danieljancar:53433-fix-error-code-beyond-expected-range
Closed

fix(core): Re-assign error codes to be within core bounds (<1000)#53455
danieljancar wants to merge 1 commit into
angular:mainfrom
danieljancar:53433-fix-error-code-beyond-expected-range

Conversation

@danieljancar

@danieljancar danieljancar commented Dec 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
Comment thread packages/core/src/errors.ts Outdated
@JeanMeche

JeanMeche commented Dec 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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

JeanMeche commented Dec 9, 2023

Copy link
Copy Markdown
Member

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

danieljancar commented Dec 11, 2023

Copy link
Copy Markdown
Contributor Author

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

JeanMeche commented Jan 26, 2024

Copy link
Copy Markdown
Member

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

danieljancar commented Jan 26, 2024

Copy link
Copy Markdown
Contributor Author

Hey, yeah is squashed. 😉

@JeanMeche

JeanMeche commented Jan 26, 2024

Copy link
Copy Markdown
Member

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
@thePunderWoman
thePunderWoman removed the request for review from jessicajaniuk April 4, 2024 20:39
@pullapprove
pullapprove Bot requested a review from alxhub April 4, 2024 20:39
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, v18-candidates Apr 5, 2024
@JeanMeche JeanMeche mentioned this pull request Apr 8, 2024
Closed
@alxhub alxhub self-assigned this Apr 11, 2024
@danieljancar

danieljancar commented Apr 11, 2024

Copy link
Copy Markdown
Contributor Author

Hi, it looks like this needs a rebase again.

Got some stuff going on right now, haven't got the time at the moment to fix this. Sorry 😿

@JeanMeche
JeanMeche force-pushed the 53433-fix-error-code-beyond-expected-range branch 2 times, most recently from 4c606a7 to 735ccf1 Compare April 11, 2024 21:02
@pullapprove
pullapprove Bot requested a review from alxhub April 22, 2024 19:05
@alxhub

alxhub commented Apr 22, 2024

Copy link
Copy Markdown
Member

Hi @danieljancar,

can you adjust your commit message to address the lint failure, and rebase this PR?

@alxhub alxhub removed this from the v18 feature freeze candidates milestone Apr 22, 2024
@ngbot ngbot Bot added this to the Backlog milestone Apr 22, 2024
@alxhub

alxhub commented Apr 22, 2024

Copy link
Copy Markdown
Member

Removing from the v18 milestone as this is not restricted to landing in v18. Feel free to make this a fix() commit.

@JeanMeche
JeanMeche force-pushed the 53433-fix-error-code-beyond-expected-range branch 2 times, most recently from d6d65af to 61d8146 Compare April 22, 2024 19:59
@JeanMeche

JeanMeche commented Apr 22, 2024

Copy link
Copy Markdown
Member

OP seemed a bit busy and since I already had to mention this issue into other PRs, I took the matter.

@danieljancar

danieljancar commented Apr 23, 2024

Copy link
Copy Markdown
Contributor Author

Thanks @JeanMeche 🙏🏻

@AndrewKushnir AndrewKushnir changed the title Reassign error codes for runtime dependencies fix(core): Re-assign error codes to be within core bounds (<1000) Apr 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

@AndrewKushnir AndrewKushnir left a comment

Copy link
Copy Markdown
Contributor

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
@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
AndrewKushnir
AndrewKushnir approved these changes Oct 2, 2024
View reviewed changes

@AndrewKushnir AndrewKushnir left a comment

Copy link
Copy Markdown
Contributor

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 approved these changes Oct 7, 2024
View reviewed changes

@thePunderWoman thePunderWoman left a comment

Copy link
Copy Markdown
Contributor

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 commented Oct 7, 2024

Copy link
Copy Markdown
Contributor

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

The changes were merged into the following branches: main

@angular-automatic-lock-bot

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

Copy link
Copy Markdown

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.

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