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

Add schematic details and links to config page #27272

Closed
wants to merge 1 commit into from

Conversation

jbogarthyde
Copy link
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?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@jbogarthyde jbogarthyde added type: bug/fix comp: docs target: patch This PR is targeted for the next patch release labels Nov 26, 2018
@ngbot ngbot bot modified the milestone: needsTriage Nov 26, 2018
@mary-poppins
Copy link

You can preview 0337411 at https://pr27272-0337411.ngbuilds.io/.

The JSON schemas for the default schematics used by the CLI to generate project components are collected in the [`@schematics` package](https://github.com/angular/angular-cli/blob/v6.0.0-rc.8/packages/%40angular/cli/lib/config/schema.json).
These schematics configure the options available to the CLI for generating the following `ng generate` sub-commands:

* [component](https://github.com/angular/angular-cli/blob/v6.0.0-rc.8/packages/%40angular/cli/lib/config/schema.json#L74-L144)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should use tags here, at least not for version 6.

But what should we use instead, thoughts?

//cc @hansl

Copy link
Contributor

Choose a reason for hiding this comment

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

The tag is fine. I don't think we should use RC tags, only stable ones.

Is there a reason this points to 6.0 instead of 7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed individual links and added description of how you could use the workspace schema.
@hansl please check.

@mary-poppins
Copy link

You can preview 7d04ba3 at https://pr27272-7d04ba3.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 29427dd at https://pr27272-29427dd.ngbuilds.io/.

@mary-poppins
Copy link

You can preview c2a61d9 at https://pr27272-c2a61d9.ngbuilds.io/.

@mary-poppins
Copy link

You can preview c24f728 at https://pr27272-c24f728.ngbuilds.io/.

@jbogarthyde jbogarthyde force-pushed the jb-config-schematics branch 3 times, most recently from a71d388 to a0a2f7f Compare November 29, 2018 16:35
@jenniferfell jenniferfell added feature Issue that requests a new feature effort2: days risk: medium and removed feature Issue that requests a new feature labels Nov 29, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 29, 2018
@mary-poppins
Copy link

You can preview 5810a3c at https://pr27272-5810a3c.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 1274cbf at https://pr27272-1274cbf.ngbuilds.io/.

@mary-poppins
Copy link

You can preview c7487ee at https://pr27272-c7487ee.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 79db1d6 at https://pr27272-79db1d6.ngbuilds.io/.

@mary-poppins
Copy link

You can preview ea07cf7 at https://pr27272-ea07cf7.ngbuilds.io/.

Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

There still seems to be some confusion here between Schematics and Architect that needs to be resolved. There are also some minor suggested improvements.

aio/content/guide/workspace-config.md Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
@jbogarthyde
Copy link
Contributor Author

jbogarthyde commented Mar 25, 2019

Thanks @Splaktar -- corrections made.

@mary-poppins
Copy link

You can preview c504609 at https://pr27272-c504609.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 54b6e63 at https://pr27272-54b6e63.ngbuilds.io/.

Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

Some additional suggestions.

At the top level of `angular.json`, a few properties configure the workspace, and a `projects` section contains the remaining per-project configuration options.
At the top level of `angular.json`, a few properties configure the workspace, and a `projects` section contains the remaining per-project configuration options. CLI defaults set at the workspace level can be overridden by defaults set at the project level, and defaults set at the project level can be overridden on the command line.

The following properties at the top level of the file configure the workspace.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The following properties at the top level of the file configure the workspace.
The following are properties at the top level of the workspace configuration file:

aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
aio/content/guide/workspace-config.md Outdated Show resolved Hide resolved
@mary-poppins
Copy link

You can preview 323792a at https://pr27272-323792a.ngbuilds.io/.

@mary-poppins
Copy link

You can preview e24620e at https://pr27272-e24620e.ngbuilds.io/.

At the top level of `angular.json`, a few properties configure the workspace, and a `projects` section contains the remaining per-project configuration options.
At the top level of `angular.json`, a few properties configure the workspace, and a `projects` section contains the remaining per-project configuration options. CLI defaults set at the workspace level can be overridden by defaults set at the project level, and defaults set at the project level can be overridden on the command line.

The following properties are at the top level of the file configure the workspace.
Copy link
Member

Choose a reason for hiding this comment

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

My whole suggestion was not accepted for this line. It still has some grammar issues even after that update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, fixing typo. This line introduces the list of top-level properties:
"The following properties at the top level of the file configure the workspace."
What do you think is wrong with it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I think that I see how you want it to read... perhaps this would make that more clear to the reader:
"The following properties, at the top level of the file, configure the workspace."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. Done.

@mary-poppins
Copy link

You can preview 7dbbc48 at https://pr27272-7dbbc48.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 58ff57f at https://pr27272-58ff57f.ngbuilds.io/.

Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

LGTM

@jbogarthyde jbogarthyde 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 Mar 27, 2019
@mary-poppins
Copy link

You can preview b6ce9a6 at https://pr27272-b6ce9a6.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 4adefc8 at https://pr27272-4adefc8.ngbuilds.io/.

@mhevery mhevery closed this in f8c7c3c Mar 27, 2019
@jbogarthyde jbogarthyde deleted the jb-config-schematics branch March 28, 2019 15:46
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
@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 Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes effort2: days risk: medium target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants