search

whphhg / vcash-electron

Multi-platform and multi-node GUI for Vcash.
GNU General Public License v3.0
38 stars 18 forks source link

Add issue and PR templates #19

Closed sum01 closed 7 years ago

whphhg commented 7 years ago

Thank you! I'll merge this as soon as I push the updated package.json and apply prettier to components. ~I've changed from Standard to Prettier.~ It achieves a better goal and makes the code uniform regardless of IDE collabolators use.

The above will require a small change in PULL_REQUEST_TEMPLATE.md from npm test to npm run prettier, but don't update it yet. I'll also try out the pre-commit hook which preformats files marked as staged, which could eliminate the npm run prettier step altogether.

I'll update this issue as soon as I'm done.

sum01 commented 7 years ago

Alright. I'll fix it whenever, just mention me on Github or Slack and I should get it done same-day.

whphhg commented 7 years ago

I forgot about standard exposing bad practices (not just style) when I wrote the above reply. I've now tested out pretty much all of https://github.com/prettier/prettier#related-projects to see which applies best.

After reading https://github.com/feross/standard/issues/811 and https://github.com/sheerun/prettier-standard/issues/13 a bunch of times I've come to the conclusion that it's best to go with prettier --no-semi --single-quote --write \"src/**/*.js\" && standard --fix for now. I've subscribed to the topics to get further updates.

The first part formats the code pretty close to standard style, the differences are quotes in react components and space after function names (most likely a couple other edge cases).

The second part goes through the now formatted code and attempts to fix those standard rules that it safely can, which fixes the two cases I mentioned above. For cases where standard --fix can't fix linting errors it will print them so the contributor can fix them manually (unused vars, adding paths instead of joining, not using ===, ...).

Please update the line below in PULL_REQUEST_TEMPLATE.md from

3. Ensure the test suite passes (`npm test`).

to

3. Format the code with prettier and lint it with standard (`npm run format`).

and I will merge it in. The npm test will be used in a future test suite, so it's best kept unused instead of confusing everyone at a later point.