Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add frontend checks and guide #447

Merged
merged 1 commit into from
Apr 27, 2021
Merged

Add frontend checks and guide #447

merged 1 commit into from
Apr 27, 2021

Conversation

GabriFila
Copy link
Contributor

@GabriFila GabriFila commented Apr 25, 2021

Description

This PR includes the necessary checks and automation to ensure proper lifting and formatting of the code:

  • prettier
  • ESLint
  • husky to run checks before local commit
  • github action to perform check

And adds general guidelines for newcomers

@kingmakerbot
Copy link
Collaborator

Hi @GabriFila. Thanks for your PR.

I am @kingmakerbot.
You can interact with me issuing a slash command in the first line of a comment.
Currently, I understand the following commands:

  • /rebase: Rebase this PR onto the master branch
  • /merge: Merge this PR into the master branch
  • /hold: Adds hold label to prevent merging with /merge
  • /unhold: Removes the hold label to allow merging with /merge
  • /deploy-staging: Deploy a staging environment to test this PR
  • /undeploy-staging: Manually undeploy the staging environment

Make sure this PR appears in the CrownLabs changelog, adding one of the following labels:

  • kind/breaking: 💥 Breaking Change
  • kind/feature: 🚀 New Feature
  • kind/bug: 🐛 Bug Fix
  • kind/cleanup: 🧹 Code Refactoring
  • kind/docs: 📝 Documentation

@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch from aa07b8a to 433b71e Compare April 25, 2021 21:08
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 26, 2021
@GabriFila
Copy link
Contributor Author

/deploy-staging

@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch from e606922 to 55ab3a2 Compare April 26, 2021 13:04
@GabriFila
Copy link
Contributor Author

/deploy-staging

@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch from 55ab3a2 to 2e139fd Compare April 26, 2021 15:29
@GabriFila
Copy link
Contributor Author

/deploy-staging

@kingmakerbot
Copy link
Collaborator

Your staging environment has been correctly deployed/updated!

@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch from 2e139fd to a84f0f2 Compare April 26, 2021 16:52
@GabriFila GabriFila requested review from giorio94, a team and QcFe April 26, 2021 17:05
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

/lgtm Thanks @GabriFila

frontend/.husky/pre-commit Outdated Show resolved Hide resolved
.github/workflows/lint.yml Outdated Show resolved Hide resolved
frontend/src/includeTailwind.css Show resolved Hide resolved
frontend/README.md Outdated Show resolved Hide resolved

The components are in `src/components`. The folder has subdirs, one for each page of the app, plus `misc` for the miscellaneous UI elements (those common between all components) and a `util` dir for custom UI elements used multiple times across the code (custom dialogs, inputs, etc...).

Each component needs to have its own folder with the following structure, e.g. for an `Exmaple` component:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Each component needs to have its own folder with the following structure, e.g. for an `Exmaple` component:
Each component needs to have its own folder with the following structure, e.g. for an `Example` component:


### File structure

Refer to the [Example component] for a demo
Copy link
Member

Choose a reason for hiding this comment

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

I suppose this should be a link


#### Component

Each component file needs to host a single components and needs to have the following structure:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Each component file needs to host a single components and needs to have the following structure:
Each component file needs to host a single component and needs to have the following structure:

Comment on lines 38 to 39
- import declarations (
When you can use non-default imports use them to reduce bundle size)
- general constant declarations for the components(if needed)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- import declarations (
When you can use non-default imports use them to reduce bundle size)
- general constant declarations for the components(if needed)
- import declarations (when you can use non-default imports use them to reduce bundle size)
- general constant declarations for the components (if needed)

frontend/README.md Outdated Show resolved Hide resolved
@giorio94 giorio94 added kind/docs Improvements or additions to documentation kind/feature New feature or request and removed kind/feature New feature or request labels Apr 26, 2021
@GabriFila GabriFila marked this pull request as ready for review April 26, 2021 17:20
@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch 7 times, most recently from 9128cf1 to 84ea9ac Compare April 27, 2021 07:10
@QcFe
Copy link
Collaborator

QcFe commented Apr 27, 2021

The package-lock.json file isn't tracked. It's usually a good thing to include it into the repository in order to ensure dependencies don't break during builds (if I got it right)

@GabriFila
Copy link
Contributor Author

The package-lock.json file isn't tracked. It's usually a good thing to include it into the repository in order to ensure dependencies don't break during builds (if I got it right)

Yes we can add it, I do it now


## Structure

Our frontend is a [React](https://it.reactjs.org/) application. We use [antd](https://ant.design/) as the main component library and [Tailwind](https://tailwindcss.com/) utilities to handle specific css scenarios (padding and margin). We also use [Storybook](https://storybook.js.org/) to ease teamwork. We chose to use [Typescript](https://www.typescriptlang.org/) to have a bettere development experience.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Our frontend is a [React](https://it.reactjs.org/) application. We use [antd](https://ant.design/) as the main component library and [Tailwind](https://tailwindcss.com/) utilities to handle specific css scenarios (padding and margin). We also use [Storybook](https://storybook.js.org/) to ease teamwork. We chose to use [Typescript](https://www.typescriptlang.org/) to have a bettere development experience.
Our frontend is a [React](https://it.reactjs.org/) application. We use [antd](https://ant.design/) as the main component library and [Tailwind](https://tailwindcss.com/) utilities to handle specific css scenarios (padding and margin). We also use [Storybook](https://storybook.js.org/) to ease teamwork. We chose to use [Typescript](https://www.typescriptlang.org/) to have a better development experience.

@GabriFila GabriFila force-pushed the gbf/add_frontend_lint branch from 84ea9ac to 910b744 Compare April 27, 2021 08:16
@GabriFila
Copy link
Contributor Author

/merge

@kingmakerbot kingmakerbot merged commit c0f4ed0 into master Apr 27, 2021
@kingmakerbot kingmakerbot deleted the gbf/add_frontend_lint branch April 27, 2021 08:37
@kingmakerbot
Copy link
Collaborator

Your staging environment has been correctly teared-down!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/docs Improvements or additions to documentation sig/devops sig/ui size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants