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(@angular-devkit/core): provide actionable warning when a workspace project has missing root property #23473

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Jun 30, 2022

The root property is required in a workspace project. Now we issue an actionable warning message when this is missing.

Note: this will become an error in the next major version.

Closes: #21310

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release 2022Q3 Fixit Candidates for the Q3 2022 fix-it labels Jun 30, 2022
@alan-agius4 alan-agius4 force-pushed the root-validation-workspace branch from 58671d3 to 404383b Compare June 30, 2022 09:04
@alan-agius4 alan-agius4 requested a review from clydin June 30, 2022 09:04
@alan-agius4 alan-agius4 force-pushed the root-validation-workspace branch from 2f633d7 to 404383b Compare June 30, 2022 11:13
@alan-agius4 alan-agius4 changed the title fix(@angular-devkit/core): provide actionable error when a workspace project has missing root property fix(@angular-devkit/core): provide actionable warning when a workspace project has missing root property Jul 7, 2022
@alan-agius4 alan-agius4 force-pushed the root-validation-workspace branch from 404383b to 6390944 Compare July 7, 2022 07:33
@alan-agius4 alan-agius4 requested review from clydin and removed request for clydin July 7, 2022 07:34
@alan-agius4 alan-agius4 force-pushed the root-validation-workspace branch from 6390944 to 86e258d Compare July 7, 2022 13:14
…e project has missing `root` property

The `root` property is required in a workspace project. Now we issue an actionable warning message when this is missing.

Note: this will become an error in the next major version.

Closes: angular#21310
@alan-agius4 alan-agius4 force-pushed the root-validation-workspace branch from 86e258d to 6f99294 Compare July 7, 2022 13:46
@alan-agius4
Copy link
Collaborator Author

@clydin PTAL

// TODO: Diagnostic reporting support
// eslint-disable-next-line no-console
console.warn(message);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Does the node provide location info? line/column info may be useful for fixing the underlying problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's actually a value, we can change this to be the actual Node quite easily, but I prefer if this was done in a separate PR.

@alan-agius4 alan-agius4 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 Jul 8, 2022
@alan-agius4 alan-agius4 merged commit 624e0b0 into angular:main Jul 8, 2022
@alan-agius4 alan-agius4 deleted the root-validation-workspace branch July 8, 2022 13:57
@angular-automatic-lock-bot
Copy link

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 Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2022Q3 Fixit Candidates for the Q3 2022 fix-it action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Angular should error with the actual config problem if project's root is missing
2 participants