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

build(web-client): add prettier #9

Merged
merged 9 commits into from
Jul 6, 2021
Merged

Conversation

PiDelport
Copy link
Collaborator

@PiDelport PiDelport commented Jun 29, 2021

@PiDelport PiDelport added the M: web-client Module: Web client app (Angular) label Jun 29, 2021
@PiDelport PiDelport self-assigned this Jun 29, 2021
@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #9 (0eec038) into main (c9a83f6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main        #9   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines            8         8           
=========================================
  Hits             8         8           
Impacted Files Coverage Δ
web-client/src/app/views/home/home.page.ts 100.00% <ø> (ø)
web-client/src/test.ts 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9a83f6...0eec038. Read the comment docs.

@PiDelport PiDelport mentioned this pull request Jun 29, 2021
5 tasks
@PiDelport PiDelport marked this pull request as ready for review June 29, 2021 15:42
@PiDelport
Copy link
Collaborator Author

@jongbonga: Does this work for your setup?

@PiDelport PiDelport mentioned this pull request Jun 29, 2021
@PiDelport PiDelport force-pushed the web-client/build-add-prettier branch from 72a0867 to 0eec038 Compare June 29, 2021 21:35
Copy link
Contributor

@jongbonga jongbonga left a comment

Choose a reason for hiding this comment

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

Prettier literally changed the whole app init linting like "Angular CLI doesn't know what they were doing by initiating the project that way, let's fix it our way which is the right way"

They are so many things in this config that are not sitting well with me...

"src/**/*.d.ts"
]
"files": ["src/test.ts", "src/polyfills.ts"],
"include": ["src/**/*.spec.ts", "src/**/*.d.ts"]
Copy link
Contributor

Choose a reason for hiding this comment

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

How can this be "more readable" than its previous state?
And will still break after a certain amount of characters or items anyway

@PiDelport
Copy link
Collaborator Author

Update from our standup discussion this morning: like @longtomjr, I don't really have any strong opinion about our formatting as long as we have something that lets us mechanise it, and not have to worry about manual formatting.

I'd lean towards using the prettier defaults just so we don't have to worry about maintaining a custom config, but I'm also happy with a customised config (or even another formatter, as long as it gives us the same convenience).

@longtomjr
Copy link
Collaborator

Just double checking @PiDelport. Is there anything holding this up still?

@PiDelport
Copy link
Collaborator Author

@longtomjr: Nope, all good!

@PiDelport PiDelport merged commit 785d169 into main Jul 6, 2021
@PiDelport PiDelport assigned PiDelport and unassigned PiDelport Jul 15, 2021
@PiDelport PiDelport deleted the web-client/build-add-prettier branch July 23, 2021 12:18
@PiDelport PiDelport removed this from the Milestone 1: Tech MVP milestone Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD M: web-client Module: Web client app (Angular)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants