Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Phonecat tutorial does not follow style guide #13312

Closed
teropa opened this issue Nov 16, 2015 · 6 comments
Closed

Phonecat tutorial does not follow style guide #13312

teropa opened this issue Nov 16, 2015 · 6 comments

Comments

@teropa
Copy link
Contributor

teropa commented Nov 16, 2015

The Phonecat tutorial does not really follow the conventions laid out by the Angular Styleguide.

In particular, I think the following rules should be applied:

It also might not hurt to explicitly refer to the style guide in the tutorial.

@teropa
Copy link
Contributor Author

teropa commented Nov 16, 2015

I'm working on the Angular 2 upgrade guide, and thinking of using Phonecat as the example app to upgrade. It would be less awkward if this issue was resolved.

I'm happy to work on this, but would first like to know whether you'd accept such changes.

@petebacondarwin
Copy link
Contributor

I agree. PRs are welcome - but phonecat is a right pain to refactor due to the nature of it using commits as the tutorial steps

@petebacondarwin petebacondarwin added this to the 1.5.x - migration-facilitation milestone Nov 16, 2015
@teropa
Copy link
Contributor Author

teropa commented Nov 16, 2015

@petebacondarwin Thanks! I'll dig into it.

teropa added a commit to teropa/angular-phonecat that referenced this issue Dec 11, 2015
Reorganize application code to be more closely aligned
with the Angular Style Guide.

This is for the final version of the app and hasn’t been
factored into steps or into the tutorial text.

1. Folders-by-feature structure: /core, /phone_list, /phone_detail
   https://github.com/johnpapa/angular-styleguide#style-y152
2. App root module depends on feature modules and core module.
   https://github.com/johnpapa/angular-styleguide#style-y165
3. One component per file. Each controller/factory/filter in its own file.
   https://github.com/johnpapa/angular-styleguide#style-y001
4. Feature+type file naming convention.
   https://github.com/johnpapa/angular-styleguide#style-y120
5. Module files, *.module.js for each module.
   https://github.com/johnpapa/angular-styleguide#style-y127
6. Component registration with getters
   https://github.com/johnpapa/angular-styleguide#style-y022
7. Named functions
   https://github.com/johnpapa/angular-styleguide#style-y024
8. Manual injection
   https://github.com/johnpapa/angular-styleguide#style-y091

Related to angular/angular.js#13312
teropa added a commit to teropa/angular-phonecat that referenced this issue Dec 11, 2015
Reorganize application code to be more closely aligned
with the Angular Style Guide.

This is for the final version of the app and hasn’t been
factored into steps or into the tutorial text.

1. Folders-by-feature structure: /core, /phone_list, /phone_detail
   https://github.com/johnpapa/angular-styleguide#style-y152
2. App root module depends on feature modules and core module.
   https://github.com/johnpapa/angular-styleguide#style-y165
3. One component per file. Each controller/factory/filter in its own file.
   https://github.com/johnpapa/angular-styleguide#style-y001
4. Feature+type file naming convention.
   https://github.com/johnpapa/angular-styleguide#style-y120
5. Module files, *.module.js for each module.
   https://github.com/johnpapa/angular-styleguide#style-y127
6. Component registration with getters
   https://github.com/johnpapa/angular-styleguide#style-y022
7. Named functions
   https://github.com/johnpapa/angular-styleguide#style-y024
8. Manual injection
   https://github.com/johnpapa/angular-styleguide#style-y091

Related to angular/angular.js#13312
teropa added a commit to teropa/angular-phonecat that referenced this issue Dec 14, 2015
Reorganize application code to be more closely aligned
with the Angular Style Guide.

This is for the final version of the app and hasn’t been
factored into steps or into the tutorial text.

1. Folders-by-feature structure: /core, /phone_list, /phone_detail
   https://github.com/johnpapa/angular-styleguide#style-y152
2. App root module depends on feature modules and core module.
   https://github.com/johnpapa/angular-styleguide#style-y165
3. One component per file. Each controller/factory/filter in its own file.
   https://github.com/johnpapa/angular-styleguide#style-y001
4. Feature+type file naming convention.
   https://github.com/johnpapa/angular-styleguide#style-y120
5. Module files, *.module.js for each module.
   https://github.com/johnpapa/angular-styleguide#style-y127
6. Component registration with getters
   https://github.com/johnpapa/angular-styleguide#style-y022
7. Named functions
   https://github.com/johnpapa/angular-styleguide#style-y024
8. Manual injection
   https://github.com/johnpapa/angular-styleguide#style-y091
9. controllerAs syntax
   https://github.com/johnpapa/angular-styleguide#style-y030

Related to angular/angular.js#13312
@teropa
Copy link
Contributor Author

teropa commented Jan 17, 2016

@petebacondarwin Looks like I'll need to make changes to some of the diagram images in the tutorial. Do you know where I might find the sources for them?

@petebacondarwin
Copy link
Contributor

Hmm. Not sure. I'll see if I can find out

@petebacondarwin petebacondarwin modified the milestones: Phonecat Tutorial Overhaul, 1.5.x - migration-facilitation Jan 25, 2016
gkalpak added a commit to gkalpak/angular.js that referenced this issue Apr 12, 2016
This is a major re-structuring of the tutorial app's codebase, aiming at applying established best
practices (in terms of file naming/layout and code organization) and utilizing several new features
and enhancements (most notably components) introduced in recent versions of Angular (especially
v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process, several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, this has been tested with the following versions of Node (on Windows 10) and everything worked
fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) angular#13834.
Again, mad props to @teropa for leading the way :)

--
**Note:**
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved on the `pre-v1.5.0-snapshot` branch of
[angular-phonecat](https://github.com/angular/angular-phonecat). The `v1.4.x` version of the
tutorial should be pointed to that branch instead of `master`.

--
Related to angular/angular-phonecat#326.
Related to angular/angular-seed#329.
Related to angular/angular-seed#333.

---
Fixes angular#12755
Fixes angular#13312
Fixes angular#13623
Fixes angular#13632

Closes angular#8952
Closes angular#11726
Closes angular#12946
Closes angular#12947
Closes angular#13198
Closes angular#13284
Closes angular#13834
Closes angular#14178
Closes angular#14223
gkalpak added a commit to gkalpak/angular.js that referenced this issue May 24, 2016
This is a major re-structuring of the tutorial app's codebase, aiming at applying established best
practices (in terms of file naming/layout and code organization) and utilizing several new features
and enhancements (most notably components) introduced in recent versions of Angular (especially
v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process, several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, this has been tested with the following versions of Node (on Windows 10) and everything worked
fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) angular#13834.
Again, mad props to @teropa for leading the way :)

--
**Note:**
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved on the `pre-v1.5.0-snapshot` branch of
[angular-phonecat](https://github.com/angular/angular-phonecat). The `v1.4.x` version of the
tutorial should be pointed to that branch instead of `master`.

--
Related to angular/angular-phonecat#326.
Related to angular/angular-seed#329.
Related to angular/angular-seed#333.

---
Fixes angular#12755
Fixes angular#13312
Fixes angular#13623
Fixes angular#13632

Closes angular#8952
Closes angular#11726
Closes angular#12946
Closes angular#12947
Closes angular#13198
Closes angular#13284
Closes angular#13834
Closes angular#14178
Closes angular#14223
@gkalpak
Copy link
Member

gkalpak commented May 25, 2016

Closed with c2033d7.

@gkalpak gkalpak closed this as completed May 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants