Skip to content

docs(aio): update ToH for CLI - #19811

Merged
vicb merged 1 commit into
angular:masterfrom
IdeaBlade:aio-cli-toh
Nov 6, 2017
Merged

vicb merged 1 commit into
angular:masterfrom
IdeaBlade:aio-cli-toh

Conversation

@Foxandxss

@Foxandxss Foxandxss commented Oct 19, 2017 •

Copy link
Copy Markdown
Member

Ready to go

@mary-poppins

Copy link
Copy Markdown

You can preview e45926b at https://pr19811-e45926b.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview 9de6a6a at https://pr19811-9de6a6a.ngbuilds.io/.

@googlebot

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@mary-poppins

Copy link
Copy Markdown

You can preview 88ff3bf at https://pr19811-88ff3bf.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview bc3d17d at https://pr19811-bc3d17d.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview 8627274 at https://pr19811-8627274.ngbuilds.io/.

@wardbell
wardbell force-pushed the aio-cli-toh branch 2 times, most recently from 0de98f9 to fb5b4a9 Compare October 25, 2017 05:25
@mary-poppins

Copy link
Copy Markdown

You can preview fb5b4a9 at https://pr19811-fb5b4a9.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview 6e8784f at https://pr19811-6e8784f.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview 8dbfb1b at https://pr19811-8dbfb1b.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview 93c0096 at https://pr19811-93c0096.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview cc444b5 at https://pr19811-cc444b5.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview ae23674 at https://pr19811-ae23674.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview d302883 at https://pr19811-d302883.ngbuilds.io/.

@mary-poppins

Copy link
Copy Markdown

You can preview d1db616 at https://pr19811-d1db616.ngbuilds.io/.

@wardbell
wardbell force-pushed the aio-cli-toh branch 2 times, most recently from fd8e1b6 to 09d23aa Compare October 27, 2017 02:22
@Nicolas-Bouteille

Copy link
Copy Markdown

Ok I see!

@Nicolas-Bouteille

Copy link
Copy Markdown
  1. HTTP >> Add HeroService.addHero()
    TSLint reports something I don't understand:
    shadowed name
    Also, what's with the 'w/' ? Does it mean something or is it a typo?

@Foxandxss

Copy link
Copy Markdown
Member Author

w/ is an english shorthand which means "with", there is also w/o which is without. We should remove that I guess.

@Nicolas-Bouteille

Copy link
Copy Markdown

ok thx

@Nicolas-Bouteille

Copy link
Copy Markdown
  1. HTTP >> Search by name.
    I was wondering why the variable heroes$: Observable<Hero[]>; ends with a dollar sign?

@Foxandxss

Copy link
Copy Markdown
Member Author

That is explained in the text.

@Nicolas-Bouteille

Copy link
Copy Markdown

Oups! Missed that! Sorry

@Nicolas-Bouteille

Copy link
Copy Markdown

Alright I'm done with the Tutorial! Yeah!

One final note that I had regarding the last section (HTTP), compared to the previous ones, it looked more like Test Driven Development. I mean that, in the previous sections, we built components and then we included them somewhere. In the last section, we include stuff that don't exist yet, so the IDE throws error, and finally when everything is done, no more errors. Just like TDD.
Personally I like TDD, but I prefer creating components first, so that the IDE autocomplete works and does not throw errors... So in the last part I have to confess I chose to create the components and methods first before including or using them. I'm not saying that the last part should change, I'm just saying that it is different from the previous sections and that I personally felt more comfortable creating stuff before mentioning them somewhere else in my code... So just a personal feedback...

Anyway, thanks a lot for this great Tutorial guys!

@wardbell

wardbell commented Nov 4, 2017 •

Copy link
Copy Markdown
Contributor

@Nicolas-Bouteille

Thank you for your most generous and thorough review. I've addressed your concerns as best I can with most recent commit in the following ways.

  1. The missing instructions to add CSS in "Hero Service". Definitely needed although not where you asked for them. There are no messages at that particular point. I've added instructions down where you modify the MessagesComponent to show messages.

  2. Fixed the incorrect import instruction in "Routing > Add routes"

  3. Added the two missing "Routing" importinstructions that you mentioned.

  4. Added the backtick comment in "Routing".

  5. I struggled with where to put the "In memory web API". I have come around to your position that it belongs in the tutorial flow. I've rewritten accordingly while striving to make clear that it has nothing to do with HttpClient and you can skip this step if you're just reading the page.

  6. Added instruction for installing the angular-in-memory-web-api npm package. I'm not sure why you get those codelyzer warnings. I don't think this package is the cause.

I would like to do something about the fact that the app keeps failing while you're building it.
It's a process of breaking and fixing.

You suggested that creating the components first would alleviate the problem. I don't think so. Adding methods to the components that refer to services that don't exist yet leaves the app broken. Jumping between component and service and back again might stop some of the breakage but not all of it and I think it would be more confusing ... certainly more confusing for someone just reading the tutorial.

Therefore, having no good answer, I'm leaving it as is for now.

Thanks again!

@mary-poppins

Copy link
Copy Markdown

You can preview 1945781 at https://pr19811-1945781.ngbuilds.io/.

@Nicolas-Bouteille

Copy link
Copy Markdown

I am quite amazed to see the reactivity of the Angular community!
Congrats...

@mary-poppins

Copy link
Copy Markdown

You can preview 67e950b at https://pr19811-67e950b.ngbuilds.io/.

@Foxandxss Foxandxss added the action: merge The PR is ready for merge by the caretaker label Nov 5, 2017
@vicb
vicb merged commit 049c896 into angular:master Nov 6, 2017
vicb pushed a commit that referenced this pull request Nov 6, 2017
@aaaaarrrgghhh

Copy link
Copy Markdown

I'm having the same error that Nicolas mentioned on nov 4 above, but I don't see a resolution of that.
its at this link in the tutorial https://angular.io/tutorial/toh-pt6#add-heroserviceaddhero

The addHero method results in TSLint complaining of a "Shadowed name: hero" for the declaration in "tap". I changed the variable name and its all fine now.
tap((hero1: Hero) => this.log(added hero w/ id=${hero.id})),

@jenniferfell

Copy link
Copy Markdown
Contributor

@mhevery Would you do me a favor and take a look at the comment above from a few days ago. The recommended change of variable name has not been made to the current released TOH.

Q1: Can you reproduce/verify the problem described in that comment? Or should I ask someone else (@kapunahelewong?)?
Q2: If the problem still exists, is the right procedure to reopen this PR or to create a new doc issue with the comment and proposed resolution?

Thanks.

@Foxandxss

Copy link
Copy Markdown
Member Author

I can't reproduce the problem

@Foxandxss
Foxandxss deleted the aio-cli-toh branch January 4, 2018 11:08
@kapunahelewong

kapunahelewong commented Jan 5, 2018 •

Copy link
Copy Markdown
Contributor

This error that @aaaaarrrgghhh mentioned sounds like it's a variable that is inadvertently declared twice. Might that be the case, @aaaaarrrgghhh? (I'm guessing as I haven't had this error before.)

@jenniferfell This would be a good candidate for a new issue if @aaaaarrrgghhh can't find the cause for the error. I can't replicate the issue on my end. Everything works as expected for me.

@aaaaarrrgghhh

Copy link
Copy Markdown

Yes, I think the variable name 'hero' is defined twice in the method
/** POST: add a new hero to the server */ addHero (**hero: Hero**): Observable<Hero> { return this.http.post<Hero>(this.heroesUrl, hero, httpOptions).pipe( tap((**hero1: Hero**) => this.log(added hero w/ id=${hero.id})), catchError(this.handleError<Hero>('addHero')) ); }

@angular-automatic-lock-bot

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 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 state: needs eng input target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants