Follow closer Angular Style Guide

[Kaido Hallik]

Hi team!

We had discussion about following closer Angular Style Guide in Angular client: https://github.com/jhipster/generator-jhipster/issues/13125

After William's comment Pascal routed this ticket to discussion in the mailing list.

Maybe two parts from this issue need discussion (according to William comment) and/or voting.
I try to make summary which is suitable for voting. For details can read discussion in https://github.com/jhipster/generator-jhipster/issues/13125


1. Move models away from shared folder in Angular client

a. leave models in shared/model/
  reasoning: as in Vue and React clients models are in shared/model/ then they should be there also in Angular client

b. move models from shared/model/ to model/
  reasoning:

    * https://angular.io/guide/styleguide#shared-feature-module - folder shared is for shared module and this should contain only shared components, directives and pipes - so models doesn't fit here

    * models can be used elsewhere in application, so better don't move under entity folder

c. generate entity models into app/entities/<entity-name>/<entity-name>.model.ts and enums to model/
  reasoning: https://angular.io/guide/styleguide#locate - code location should be intuitive, simple and fast - currently if I work with entity and want to look into it's model file then I must do extra effort to find it because it's not under entity folder


2. Move unit tests next to files they are testing

a. leave tests in src/test/javascript/spec
  reasoning:
    * as in Vue and React side tests are there then tests should be there also in Angular client
    * developers with Java backend are used to the fact that tests are separated, so for them it's familiar to see tests separated also in Angular client

b. Move unit tests next to files they are testing and delete src/test/javascript/spec after that
  reasoning:
    * follow Angular best practice as described in: https://angular.io/guide/testing#place-your-spec-file-next-to-the-file-it-tests - I agree with all those arguments
    * as Angular CLI generates tests next to files they are testing then currently we have turned off tests generation in Angular CLI; I think this is very bad to our reputation, this leaves impression like we think that unit testing is unimportant, which is wrong of course.
    * Angular examples in the web are following this Angular best practice, after this change application generated by us is with similar structure as those examples

In the morning I already started working with this 2. point and I finished this work in branch https://github.com/kaidohallik/generator-jhipster/tree/refactor-angular-tests
So if someone wants to see how it looks if unit tests are next to files they are testing then this branch generates such Angular client.

With best regards,

Kaido.

[Deepu K Sasidharan]

Hi,

For item 1, I'm ok to move models. Originally I think I did the current folder structure based on whatever was Angular style guide at the time it was done. I also did duck typing back then based on the style guide (very bad idea) 

Another reason for the current structure is also the imports. Since we have a lot of dynamic templated imports, the current structure allowed us to reduce template complexities to an extent. so if the new style guide structure doesn't make imports way too complicated to achieve in templates (especially when using microservices where folder structure is organized by service) then I'm ok for it. One of our goal was also to align to the framework/libs style guide where possible.

For item 2, I vote to keep the current format, while I personally prefer tests in the same folder as the source for client-side it might make it difficult for backend devs who are our major users and also will make things inconsistent. Keep in mind that another goal of ours is to provide a similar experience and structure regardless of what front/back tech you use. SO if we move the location of front end tests, we need to do it for React and Vue as well.

Thanks & Regards,

Deepu

--
You received this message because you are subscribed to the Google Groups "JHipster dev team" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jhipster-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jhipster-dev/em953c6abe-a2fa-4e8d-9947-cdfc0f5f2027%40sille.

[Marcelo Boveto Shima]

Thanks Kaido for bringing this discussion.

But I think we should change the test placement discussion.

- At CONTRIBUTING.md: Angular Typescript files must follow the Official Angular style guide.

- Angular style guide suggests to Place your spec file next to the file it tests.

The discussion that follows is:

1. Should we create a jhipster style guide for test placements?

If so we should update the contributing guide.

In case of 1. Yes:

2. Official blueprints have to follow the jhipster style guide?

In case of 2. No:

2. In case of a missing official style guide for the technology?

2.a. Should fallback to angular style guide (for clients).

2.b. A team should define the style.

2.c. Style should be defined at mail list.

We generate source code, not binary, so IMO we should follow the technology development best practices.

1. No

2.b

Regards.

Marcelo.

[Marcelo Boveto Shima]

I don’t agree it would "might make it difficult for backend devs who are our major users".

- "might make it difficult for backend devs": A backend dev would look for official guides and tutorials that recommends putting tests next to to test target.

  Following the official best practices is better IMO.

- "who are our major users": IMO jhipster makes much more sense as a backend tool than a frontend tool.

  So a frontend developer would generate only backend and can be confused as a backend developer.

Regards

Marcelo.

To view this discussion on the web visit https://groups.google.com/d/msgid/jhipster-dev/CAFXry55CdkQpwg9cKncvkXsm%2BEupjb8ry1XPoZMh6eyxi8cS7A%40mail.gmail.com.

[Pascal GRIMAUD]

Hello,

Same vote as Deepu.

Item 1: ok to move, if needed

Item 2: I'd prefer to keep like this, separate code and tests

Pascal

To view this discussion on the web visit https://groups.google.com/d/msgid/jhipster-dev/C647BEDE-BB38-4C14-A668-97CD7C46CC18%40gmail.com.

[William Marques]

Hi,

Item 1:

I'm ok to move the entities under the entity folder or not move them at all. I don't see the point of having a model folder at the root of the project.

Item 2:

I have mixed feelings, I think it's more consistent to have them in a separate folder if we think of a monolith application.

However, I think having tests next to source code is really useful and reminds the developer to write unit tests :-)

So I would vote for moving them next to source code.

As Deepu said, if we do it then we should do the same for React and Vue.

Regards,

William

To view this discussion on the web visit https://groups.google.com/d/msgid/jhipster-dev/CALUG8VvBU8vTdXAwGLbr0gO0w63mfwqYv7SGXUbY0ERa%3D2Ug%3DA%40mail.gmail.com.

[Daniel Franco]

Hi,

Item 1: OK to move

Item 2: since we switch now to Angular CLI, to be consistent I would prefer to keep tests on the same folder.

To view this discussion on the web visit https://groups.google.com/d/msgid/jhipster-dev/CAKVL0B2EGRjatUEx%2BMWFoTw_uPPAHthQYUB3kiGMLB%2B1Ab7Eug%40mail.gmail.com.