lablup / backend.ai-webui

Backend.AI Web UI for web / desktop app (Windows/Linux/macOS). Backend.AI Web UI provides a convenient environment for users, while allowing various commands to be executed without CLI. It also provides some visual features that are not provided by the CLI, such as dashboards and statistics.
https://www.backend.ai
GNU Lesser General Public License v3.0
110 stars 73 forks source link

Let's provide code convention guide and apply rules to config files. #1382

Open lizable opened 2 years ago

lizable commented 2 years ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] 기능 요청이 특정 문제에 연관된 것이라면 여기에 그 문제를 설명해주세요. It's time to provide a newbie-friendly guide. Code convention guide would be a good example. There are lots of code conventions that are only exposed in code reviewing. There are two benefits of defining code conventions:

Describe the solution you'd like A clear and concise description of what you want to happen. 어떤 기능이 있으면 좋겠는지 자세히 설명해주세요.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered. 혹시 다른 대안들을 생각해본 적이 있다면 함께 적어주세요. We may just use "JS standard style", if all of the works seem in vain...

Additional context Add any other context or screenshots about the feature request here. 기능 요청에 대해 보다 잘 이해할 수 있는 다른 맥락을 기술해주세요.

lizable commented 2 years ago

Also, there will be code refactoring chores after the code convention is defined. Such as unifying using Camel case or Kebab case, etc.

Jaewoook commented 2 years ago

I found a PR #1306 that updates lint rules. How's it going?

lizable commented 2 years ago

I found a PR #1306 that updates lint rules. How's it going?

We need to dig and polish (manually, if possible) rules, since it's not perfectly fit into our expectations.

Jaewoook commented 2 years ago

I would like to suggest some conventions.

Suggestion: Use consistent variable name

Although the suggestion is ambiguous, I'm going to handle some case when naming variable.

1. element reference variable

In page components, there are so many variable referencing html element, so developer cannot determine what element variable references what element. For example, let's suppose there is a variable named someCount, the someCount variable can be select and input element reference. Then how about someCountSelect or someCountInput? Adding element type suffix is more clear to determine what element is.

2. element naming convention

There are many way to naming variable like kebab-case, camelCase, snake_case, PascalCase, etc. We can choose naming convention to naming variable and HTML classes, ids, and more. My suggestion is using camelCase in class property / method, kebab-case in html class / id to naming it. I think there will be some edge case like vfolder. In this case, how about capitalize the shorten letter and also next letter, like VFolder?

Any feedback, opinion are welcome 😃

Jaewoook commented 2 years ago

Suggestion: import order

I suggest applying the import/order rule. You can read more detail in following link.

rule documentation: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md

Jaewoook commented 2 years ago

Suggestion: lit-a11y rules

The lit-a11y (accessibility) rules already applied in this project but there's so many lit-a11y errors in this project. Good accessibility makes better web experience for user. Let's follow the lit-a11y rules strictly.

Jaewoook commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics

Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

lizable commented 2 years ago

I would like to suggest some conventions.

Suggestion: Use consistent variable name

Although the suggestion is ambiguous, I'm going to handle some case when naming variable.

1. naming element reference variable

In page components, there are so many variable referencing html element, so developer cannot determine what element variable references what element. For example, let's suppose there is a variable named someCount, the someCount variable can be select and input element reference. Then how about someCountSelect or someCountInput? Adding element type suffix is more clear to determine what element is.

2. element naming convention

There are many way to naming variable like kebab-case, camelCase, snake_case, PascalCase, etc. We can choose naming convention to naming variable and HTML classes, ids, and more. My suggestion is using camelCase in class property / method, kebab-case in html class / id to naming it. I think there will be some edge case like vfolder. In this case, how about capitalize the shorten letter and also next letter, like VFolder?

Any feedback, opinion are welcome 😃

I agree. I've tentatively applied the first suggestion on here(#1385). One more addition to the first suggestion, I would also like to suggest adding a ~List suffix when the member variable type is Array<T>. And for element naming conventions, let's sum up the edge cases (like VFolder) somewhere so that we can find the patterns and apply the alternative rules.

Jaewoook commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics

Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component
    • variable declaration
    • with initializer
    • operation logics
    • event handler
    • render
    • sub-rrender
    • main render

Other component structure suggestion:


Event handler after render function


variable initializer and operation logics after render and handlers

agatha197 commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics

Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-render

    • main render

Good suggestion. In addition to your opinion, I suggest placing style codes right before the main render codes. In some files, the style code is after the constructor, and in some files before the main render. I want to unify this, too.

Jaewoook commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics

Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-rrender

    • main render

We can use this rule to achieve our goals

documentation link: https://typescript-eslint.io/rules/member-ordering/

Jaewoook commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-render

    • main render

Good suggestion. In addition to your opinion, I suggest placing style codes right before the main render codes. In some files, the style code is after the constructor, and in some files before the main render. I want to unify this, too.

How about split style code to component's outside? style code annoys me when I read the javascript source.

I think there are two methods we could try:

or separate files like this

lizable commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-render

    • main render

Good suggestion. In addition to your opinion, I suggest placing style codes right before the main render codes. In some files, the style code is after the constructor, and in some files before the main render. I want to unify this, too.

How about split style code to component's outside? style code annoys me when I read the javascript source.

I think there are two methods we could try:

  • class

    • class sources
  • styles

or separate files like this

  • components/

    • some-page/

    • index

    • some-page-component

    • some-page-style

https://lit.dev/docs/components/styles/#external-stylesheet Perhaps this may help to figure out the best way to handle styles

Jaewoook commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-render

    • main render

Good suggestion. In addition to your opinion, I suggest placing style codes right before the main render codes. In some files, the style code is after the constructor, and in some files before the main render. I want to unify this, too.

How about split style code to component's outside? style code annoys me when I read the javascript source. I think there are two methods we could try:

  • class

    • class sources
  • styles

or separate files like this

  • components/

    • some-page/

    • index

    • some-page-component

    • some-page-style

https://lit.dev/docs/components/styles/#external-stylesheet Perhaps this may help to figure out the best way to handle styles

'link styles externally' dost not mean using tag. We can just import styles and set it to class's static property like this.

SomeStyles.ts


import {css} from 'lit';

export const outsideStyles = [ css/** ...styles go here */; ];


> SomeComponent.ts
```ts
import {outsideStyles} from './styles-from-outside';

class SomeComponent extends LitElement {
    static get styles() {
        return outsideStyles;
    }
}
lizable commented 2 years ago

Suggestion: component method order (component structure)

I think component can be separated as 4 part - variables / event handlers / renders / operation logics

Each component has different order of this parts. Consistent order makes better readability for developer to find what they want to see.

I suggest this component structure:

  • class Component

    • variable declaration

    • with initializer

    • operation logics

    • event handler

    • render

    • sub-render

    • main render

Good suggestion. In addition to your opinion, I suggest placing style codes right before the main render codes. In some files, the style code is after the constructor, and in some files before the main render. I want to unify this, too.

How about split style code to component's outside? style code annoys me when I read the javascript source.

I think there are two methods we could try:

  • class

    • class sources
  • styles

or separate files like this

  • components/

    • some-page/

    • index

    • some-page-component

    • some-page-style

https://lit.dev/docs/components/styles/#external-stylesheet Perhaps this may help to figure out the best way to handle styles

'link styles externally' dost not mean using tag. We can just import styles and set it to class's static property like this.

SomeStyles.ts


import {css} from 'lit';

export const outsideStyles = [

    css`/** ...styles go here */`;

];

SomeComponent.ts


import {outsideStyles} from './styles-from-outside';

class SomeComponent extends LitElement {

    static get styles() {

        return outsideStyles;

    }

}

Okay, I get it. We've already applied this kind of code splitting in backend-ai-general-styles component (and other components ends with ~styles), so refactoring current compnents especially on styles definition could be as simple as that. (though we have more than 50 components to split, 🥲)