Skip to content

Format frontend projects with prettier#2769

Merged
BacLuc merged 6 commits into
ecamp:develfrom
manuelmeister:feature/cleanup
Jun 13, 2022
Merged

Format frontend projects with prettier#2769
BacLuc merged 6 commits into
ecamp:develfrom
manuelmeister:feature/cleanup

Conversation

@manuelmeister

@manuelmeister manuelmeister commented May 29, 2022

Copy link
Copy Markdown
Member

Fixes #2749

@manuelmeister manuelmeister force-pushed the feature/cleanup branch 4 times, most recently from e43be6b to 92c1164 Compare May 29, 2022 12:37
@manuelmeister manuelmeister requested review from BacLuc and usu May 29, 2022 12:41
Comment thread common/locales/de-CH-scout.json Outdated
Comment thread frontend/package.json Outdated
Comment thread frontend/src/App.vue Outdated

@carlobeltrame carlobeltrame left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we also give some formatting love to the react components?

@manuelmeister

Copy link
Copy Markdown
Member Author

@carlobeltrame

Copy link
Copy Markdown
Member

Ah! My phone didn't show me all changed files I think. Looks great 👍

@BacLuc

BacLuc commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

When i run lint:check locally, i get >5000 problems.
When i run lint, then the following is the output: manuelmeister#2
But the github actions verifies the output.
Is it just me, can someone verify mine or the github action result of the formatting?

And the github actions also passes on my changes...? https://github.com/BacLuc/ecamp3/runs/6810618861?check_suite_focus=true

@manuelmeister

Copy link
Copy Markdown
Member Author

This could be connected to https://prettier.io/docs/en/rationale.html#multi-line-objects and a different printWidth somehow set.

Comment thread .prettierrc
@BacLuc

BacLuc commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

When i run lint:check locally, i get >5000 problems. When i run lint, then the following is the output: manuelmeister#2 But the github actions verifies the output. Is it just me, can someone verify mine or the github action result of the formatting?

And the github actions also passes on my changes...? https://github.com/BacLuc/ecamp3/runs/6810618861?check_suite_focus=true

I thought i understand eslint and prettier, but i don't.
If i run into problems with prettier, i will fix them

@manuelmeister manuelmeister force-pushed the feature/cleanup branch 2 times, most recently from c9f130a to 1c35f41 Compare June 12, 2022 20:10
@manuelmeister

Copy link
Copy Markdown
Member Author

@BacLuc does this work for you now?

@BacLuc BacLuc merged commit 4d90072 into ecamp:devel Jun 13, 2022
@usu

usu commented Jun 13, 2022

Copy link
Copy Markdown
Member

🙄 ui, many merge conflicts this will create...

I'm not sure this PR works as expected. I tried to merge into #2683, with the intention to accept all my changes in the merge conflicts and then re-run lint. Following is observed:

  • After resolving merge conflicts, running npm run lint locally doesn't change anything
  • Auto Format in VS Code doesn't change anything
  • However, code style clearly doesn't follow this PR's code style (see for example on this line )
  • running npm run lint:check locally in frontend or print folder says "no issues found"
  • running docker-compose exec frontend npm run lint:check finds issues in 2 files (global.scss and variables.scss)

@manuelmeister

manuelmeister commented Jun 13, 2022

Copy link
Copy Markdown
Member Author

Sorry for the inconvenience, I did intend for you to review before merging.
Maybe @BacLuc can help you improve the configuration for VS Code.

As for the rest, I'm not sure why the linting has no effect. It could be an installation issue. Because yesterday when I pulled develop into this, and forgot to do the linting, it correctly showed the same errors as local.

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.

Add frontend code formatter prettier

4 participants