Skip to content

Conversation

@jcamposmk
Copy link
Contributor

@jcamposmk jcamposmk commented Sep 6, 2021

Guide Lines

task: https://makingsense.atlassian.net/browse/DW-1087

  • Recommendations for testing
  • Reccomendations for code organization
  • Others recommendations

RESULT:

Guide Lines

Test

  • It's recommended to test from the user's point of view, hiding implementation details. More details.

  • Recommended documentation for test implementation.

  • Test phases:

    • Arrange: a series of input data is created to be able to run the tests.

    • Act: tests are run with the input data.

    • Assert: the result obtained is analyzed to see if it matches what is expected.

  • Pre-commit hook is used in local development (husky+link-staged). More details.

  • There's no QA team, so each development must be tested. Also, if a new development is not tested, coverage decreases. If the coverage is below the accepted limit, the merge will not be possible. More details.

Code organization

  • Components: This is located in the src/components path. Each should be located in a separate folders and must contain:

    • component implementation

    • tests

    • styles

    • images

    Also, the component is organized internally in the following order:

    • hooks are located at the top

    • constants and states together

    • functions.

    Outside the component the variables or functions that do not depend on some props. See slack conversation thread for more details.

  • Hooks: This located in src/hooks path. Each should be located in a separate folders and must contain:

    • hook implementation

    • tests

  • Services: This is located in the src/services path. Each should be located in a separate folder and must contain:

    • service implementation

    • tests

    • service double

  • Translations: This is located in src/i18n path. There are two files: en.js and es.js. If the translation is common, it's added within the common property, otherwise, it's added in the property that contains the translations of the corresponding module.

  • Gif/Images: The reused gif/images are located in the src/img path. The particular images are located in the folder of the component that uses them. Example: a gif for the contact policy promotional page.

Procure to use declarative style instead of the imperative style

  • Use const instead of let and var. Example:

    // Not recommended
    let result;
    if(someCondition) {
      result = calculationA();
    } else {
      result = calculationB();
    }
    
    // Better
    const result = someCondition ? calculationA() : calculationB();
    

    Reference documentation.

  • Use High Order Functions when it’s possible. Examples:

    // Not recommended 
    for(let i=0; i < cities.length; i++) {
      const city = cities[i];
      // to do something with city
    } 
    
    // Better
    cities.forEach((city) => {
      // to do something with city
    });
    
    // Not recommended 
    for(let i=0; i < myArray.length; i++) {
      myArray.key = “a value”;
      myArray.otherKey = “a value”;
    } 
    
    // Better
    const newArray = myArray.map((item) => ({
      ...item,
      key: “a value”,
      otherKey: “a value”,
    }));
    

Others

  • Use composition instead of inheritance to reuse code between components. More details.

  • Use named imports instead of default imports. More details.

  • Use pronounceable and expressive names for variables, always prioritizing the camelcase style. Avoid using names that refer to the data type.

  • Use the useTimeout hook on components instead of setTimeout. More details.

  • Use a maximum number of 3 parameters for functions. If the number is exceeded, use an object.

  • Use the optional chaining operator. Example:

    // Not recommended
    if(obj.first && obj.first.second) { 
      // ...to do something 
    }
    
    // Better
    if(obj?.first?.second) {
      // ...to do something
    }
    

README.md Outdated
### Code organization
- **Components:** This is located in the *src/components* path. Each should be located in separate folders, each one must contain: *component implementation, tests, styles and images*. Also, the component is organized internally in the following order: hooks are located at the top, constants and states together in one place, next are the functions. Outside the component the variables or functions that do not depend on some props. [See slack conversation thread for more details](https://makingsense.slack.com/archives/CGJ72QFQX/p1625754680182000).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more clear this way:

Suggested change
- **Components:** This is located in the *src/components* path. Each should be located in separate folders, each one must contain: *component implementation, tests, styles and images*. Also, the component is organized internally in the following order: hooks are located at the top, constants and states together in one place, next are the functions. Outside the component the variables or functions that do not depend on some props. [See slack conversation thread for more details](https://makingsense.slack.com/archives/CGJ72QFQX/p1625754680182000).
- **Components:** This is located in the *src/components* path. Each should be located in separate folders, each one must contain: *component implementation, tests, styles and images*. Also, the component is organized internally in the following order:
- hooks are located at the top
- constants and states together
- functions.
**Outside the component** the variables or functions that do not depend on some props. [See slack conversation thread for more details](https://makingsense.slack.com/archives/CGJ72QFQX/p1625754680182000).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **Components:** This is located in the *src/components* path. Each should be located in separate folders, each one must contain: *component implementation, tests, styles and images*. Also, the component is organized internally in the following order: hooks are located at the top, constants and states together in one place, next are the functions. Outside the component the variables or functions that do not depend on some props. [See slack conversation thread for more details](https://makingsense.slack.com/archives/CGJ72QFQX/p1625754680182000).
- **Components:** This is located in the *src/components* path. Each should be located in separate folders, each one must contain: *component implementation, tests, styles and images*. Also, the component is organized internally in the following order:
- hooks are located at the top
- constants and states together
- functions.
**Outside the component** the variables or functions that do not depend on some props. [See slack conversation thread for more details](https://makingsense.slack.com/archives/CGJ72QFQX/p1625754680182000).

I've forgot to add an extra level for bullets 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, ahora mismo se está respetando todo esto?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bueno, ahora mismo no veo múltiple niveles de identación.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Estamos intentando ir hacia ahí, en cada PR es algo que vamos revisando, si hay que acomodar varios componentes que no respetan ese formato. A eso hacías referencia @andresmoschini ?

@jcamposmk jcamposmk force-pushed the DW-1087-guide-lines branch 2 times, most recently from 8913fb9 to f4411b0 Compare September 7, 2021 16:32
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 7, 2021
@cbernat
Copy link
Contributor

cbernat commented Sep 7, 2021

I think this is ready to merge, all other things that may be missing we can add them later. Good job @jcamposmk ! 🚀

@jcamposmk jcamposmk merged commit 01ce0ff into FromDoppler:master Sep 7, 2021
@andresmoschini
Copy link
Contributor

🎉 This PR is included in version 1.251.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants