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

Prettify Solidity files #1623

Merged
merged 3 commits into from
Nov 7, 2019
Merged

Prettify Solidity files #1623

merged 3 commits into from
Nov 7, 2019

Conversation

nambrot
Copy link
Contributor

@nambrot nambrot commented Nov 6, 2019

Description

As a constant violator of the Solidity Style Guide (@asaj can attest to this fact), I told myself: "No more!". Hence this PR adds prettier-solidity which will prettify our .sol files so that future generations shall no longer need to live in fear of style guide violations!

It was actually incredibly easy, there was only 1 place where I needed to add a // prettier-ignore

Tested

  • CI

Backwards compatibility

  • I believe they should be

@nambrot
Copy link
Contributor Author

nambrot commented Nov 7, 2019

@asaj I looked at the linked override at https://github.com/prettier-solidity/prettier-plugin-solidity#vscode, I wasn't sure if that's what we wanted, for example a width of 80 (instead of the 100) that we have been using. Both numbers seemed appropriate, so I kept 100 as it was less churn. Same with tabs/spaces, just kept the default

@asaj
Copy link
Contributor

asaj commented Nov 7, 2019

@asaj I looked at the linked override at https://github.com/prettier-solidity/prettier-plugin-solidity#vscode, I wasn't sure if that's what we wanted, for example a width of 80 (instead of the 100) that we have been using. Both numbers seemed appropriate, so I kept 100 as it was less churn. Same with tabs/spaces, just kept the default

+1 to sticking with our defaults here

@asaj asaj assigned nambrot and unassigned asaj Nov 7, 2019
@nambrot nambrot added the automerge Have PR merge automatically when checks pass label Nov 7, 2019
@codecov
Copy link

codecov bot commented Nov 7, 2019

Codecov Report

Merging #1623 into master will decrease coverage by 0.28%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1623      +/-   ##
==========================================
- Coverage   74.15%   73.86%   -0.29%     
==========================================
  Files         287      283       -4     
  Lines        7652     7537     -115     
  Branches      960      954       -6     
==========================================
- Hits         5674     5567     -107     
+ Misses       1865     1858       -7     
+ Partials      113      112       -1
Flag Coverage Δ
#mobile 73.86% <ø> (-0.29%) ⬇️
Impacted Files Coverage Δ
packages/mobile/src/escrow/saga.ts 19.59% <0%> (-1.36%) ⬇️
packages/mobile/src/utils/formatting.ts 89.13% <0%> (-0.67%) ⬇️
...bile/src/paymentRequest/PaymentRequestListItem.tsx 72.34% <0%> (-0.58%) ⬇️
...es/mobile/src/verify/VerificationLoadingScreen.tsx 100% <0%> (ø) ⬆️
packages/mobile/src/components/Carousel.tsx 88.88% <0%> (ø) ⬆️
packages/mobile/src/navigator/Screens.tsx 100% <0%> (ø) ⬆️
...s/mobile/src/paymentRequest/NotificationAmount.tsx 100% <0%> (ø) ⬆️
...paymentRequest/PaymentRequestNotificationInner.tsx 100% <0%> (ø) ⬆️
packages/mobile/src/navigator/Headers.tsx 100% <0%> (ø) ⬆️
packages/mobile/src/analytics/constants.ts 100% <0%> (ø) ⬆️
... and 10 more

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 6c5d794...a4f2498. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants