-
Notifications
You must be signed in to change notification settings - Fork 11.9k
docs(@angular-cli): update documentation for ng new
#6480
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
Conversation
Could you also update the command documentation as well? Thanks! https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/commands/new.ts |
@delasteve where can I find your docs standards for code? |
I was merely asking to update the descriptions on the commands. For example, https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/commands/new.ts#L90 |
Got it. |
3336151
to
465fa86
Compare
@delasteve not used to this whole github ecosystem (we use gerrit at work, not working open source), but I've managed to figure this whole deal out. updated the command descriptions as well |
858c86a
to
30d8a0a
Compare
Can I get a review on this? |
The style file default extension. Possible values: | ||
<ul> | ||
<li>css</li> | ||
<li>scss</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styl
is also possible for stylus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well as less
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I have to add to both the docs and the command: styl
and less
? what about sass
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as far as I can see, from commands/new.ts
up until blueprints/ng/index.ts
, there's only validation for stylus
to change the extension to styl
. Meaning that anyone can f.e put asdf
and that would work. I also don't see any e2e testing for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm wrong and there's validation for that, please tell me. If not, I guess that's something that you should consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@filipesilva I've updated the PR commit with less and stylus as options. I've stated styl
for stylus
, but I've written styl (stylus)
to make it clear that it's clear.
@sumitarora please take a look. |
7d7fefe
to
df1ed8f
Compare
36b56ff
to
5d28188
Compare
@sumitarora can I get a review on this? It would help me if this goes in, as I want to add similar docs in my other PR |
@gioragutt LGTM, thanks! |
@filipesilva when will this be merged to master? does this happen only on release? or is it something I have to do? I'm quite new to this, so I'd like some explanation if it's possible. And also any constructive criticism is always appreciated :) |
@filipesilva just noticed this was merged. Thanks a lot! |
It's in |
Oh duh, sorry, yeah it was approved to be merged but hasn't been yet. My bad. |
@filipesilva cool, I was worried for a second :P I'm pulling from master but nothing new is coming. will the PR automatically close on merge? |
Yes |
@filipesilva by the way, i'm gonna update the commit - I've seen a tutorial on youtube where the dude uses |
* for several options that are saved in .angular-cli.json, I've added an explanation of what setting in the json file controls that options. * added an explanation about what --dry-run will output. * added possible values for --style option * added respective documentation in the command * also added less, sass and styl(stylus) as possible style extensions The motivation for this change is that sometimes people would want to change some of the settings set by the cli durig ng new, that they might have not known or cared about, for example, prefix.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
.angular-cli.json
, I've added an explanation of what setting in the json file controls that options.--dry-run
will output.--style
optionThe motivation for this change is that sometimes people would want to change some of the settings set by the
cli
durigng new
, that they might have not known or cared about, for example,prefix
.