Project

General

Profile

Bug #34312

mgr/dashboard: Style guide for frontend/Angular coding conventions

Added by Patrick Seidensal over 5 years ago. Updated almost 3 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
General
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Let's start the discussion about coding conventions as it seems we don't have at least the following topic covered yet or no clear coding conventions at all. Please feel free to add topics to be discussed here as well as your opinion on the topic mentioned below.

I have noticed that the models seems to be everywhere in the code (which might make sense, but I don't know if it makes sense). There's a central shared location for models and there are models in module and component directories. The models have very different names. Some indicate that they are models by having a `.model.` string in their name, others don't. This together makes it hard to find particular models or even get a list of available/existing models to be reviewed or used.

I personally feel that, if it makes sense to have models basically everywhere, we should at least stick to a naming convention so that they can easily be found. `.model.` feels natural to me as components and services follow this convention, too. What are your thoughts about that and other topics?


Related issues

Related to Dashboard - Feature #27218: mgr/dashboard: Style guide to give a the UI an overall look and feel Resolved
Related to Dashboard - Feature #34530: mgr/dashboard: CdSubmitButton should be disabled if the form was not modified Closed
Related to Dashboard - Cleanup #35683: mgr/dashboard: Refactoring of Modal Dialogs New

History

#1 Updated by Stephan Müller over 5 years ago

I have a view topics that aren't covered.

File / Method lengths?
INHO less than 500 lines per file which raises the question where to split it up and how to (should be explained to).
Method less than 20 lines.

Should Methods be allowed to contain newlines?
IMHO they should not.

#2 Updated by Patrick Seidensal over 5 years ago

Another topic which came up recently is the underscore prefix for private methods. I've seen it in a pull request and suggested to completely not use it anymore for various reasons (which I didn't mention) and because the Angular Style Guide says "don't do that" (which I did mention).

#3 Updated by Patrick Seidensal over 5 years ago

  • Subject changed from mgr/dashboard: Style guide for frontend/Angualar coding conventions to mgr/dashboard: Style guide for frontend/Angular coding conventions

#4 Updated by Patrick Seidensal over 5 years ago

I forgot to mention that I would generally like to stick to the Angular Style Guide and only add rules where we think its required (project specific additions). I think project specific exceptions should be rare but we might want or need them, too.

#5 Updated by Patrick Seidensal over 5 years ago

codelyzer might be helpful as it is compatible with Angular and extends tslint rules to match pre-defined rules of the Angular Style Guide.

#6 Updated by Lenz Grimmer over 5 years ago

Note that Ceph has a dedicated CodingStyle document that already has a section about JavaScript / TypeScript - this is the document that should be updated with any additional conventions that we agreed to.

#7 Updated by Lenz Grimmer over 5 years ago

Patrick Nawracay wrote:

I forgot to mention that I would generally like to stick to the Angular Style Guide and only add rules where we think its required (project specific additions). I think project specific exceptions should be rare but we might want or need them, too.

FYI: the aforementioned Coding Style document already states that we follow the official Angular Style Guide.

#8 Updated by Lenz Grimmer about 5 years ago

  • Related to Feature #27218: mgr/dashboard: Style guide to give a the UI an overall look and feel added

#9 Updated by Lenz Grimmer about 5 years ago

  • Related to Feature #34530: mgr/dashboard: CdSubmitButton should be disabled if the form was not modified added

#10 Updated by Lenz Grimmer about 5 years ago

  • Related to Cleanup #35683: mgr/dashboard: Refactoring of Modal Dialogs added

#11 Updated by Ernesto Puerta almost 4 years ago

Reviewed and decided to keep it open

#12 Updated by Ernesto Puerta almost 3 years ago

  • Project changed from mgr to Dashboard
  • Category changed from 132 to General

Also available in: Atom PDF