gun

gun copied to clipboard

So just to clarify, its not about forcing a coding standard down on you ~ #227

To justify why I feel linting would assist this project:

• Linting gives you suggestions while you are coding to identify issues
• Best practices actually improve not only security but performance in some cases
• Catches common errors that sometimes take ages to find, due to a missing comma/bracket/etc

But most important, so I will stipulate this separately. Linting is essential for code review!

Also this project has files which do not parse as valid JavaScript, currently npm run lint actually returns errors that have nothing to do with styling.

And you will find this is the most forgiving/allowing code linting rules in any project.

I have actually disabled the majority of the linting rules so that you can opt-in rather than deal with the massive task of bringing this to up to date with best practices.

Also I see you have massive branches you need to merge so...

That being said I feel there are some rules which are disabled that as I said are essential imho

• https://eslint.org/docs/rules/no-mixed-spaces-and-tabs
• https://eslint.org/docs/rules/indent
• https://eslint.org/docs/rules/linebreak-style
• https://eslint.org/docs/rules/semi
• https://eslint.org/docs/rules/eol-last

I am happy to take a phased approach at this point, without linting I feel like I am walking drunk into an African brothel without a condom 🤪

This will effect your development workflow, with the addition of the pre-commit and pre-push git hooks. You should enable editorconfig and eslint plugins in your IDE to guide you to not have issues when it comes time to commit or push.

@expelledboy no doubt this is required:) maybe also push the files after lint, otherwise people will start having merge conflicts if we in did start using this

So I can introduce this as both a pre-commit hook and into npm test, but could I get assistance with some of the errors of npm lint?

I dont want to make too many changes to introduce linting, and I am not confident yet with the codebase.

Okay so I disabled even more checks 🤣 well yolo, we can bring in more rules as and went we don't have large branches to merge.

Nice CI working with npm lint

https://travis-ci.org/github/amark/gun/builds/742923538#L230

Am I pushing it too far to also add code coverage limit of 65% while I am at it? :)

You guys actually have done a solid job so far! I wasnt expecting to set to all the way up to 65% when the project didnt actually include coverage reports.

---------------|---------|----------|---------|---------|---------------------------------------------------------------
File           | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------|---------|----------|---------|---------|---------------------------------------------------------------
All files      |   65.75 |    56.94 |   64.55 |   69.32 |
 gun           |   64.62 |    56.92 |   63.43 |   67.69 |
  gun.js       |   66.78 |    59.35 |    63.5 |   69.59 | ...870-1871,1946-2084,2098-2361,2368-2377,2382-2386,2412-2450
  sea.js       |    60.4 |    51.29 |   63.29 |   64.29 | ...221,1231,1234,1237-1239,1243-1245,1250,1255,1280,1290-1438
 gun/lib       |   69.76 |    57.01 |   69.23 |   75.14 |
  fsrm.js      |   84.62 |    66.67 |     100 |   91.67 | 12
  radisk.js    |   67.64 |    53.65 |   75.81 |   73.18 | ...01-426,429-437,474-475,479-481,489,512-531,536-560,566-568
  radix.js     |   91.07 |    89.34 |      75 |   94.94 | 93-94,108-109
  radmigtmp.js |      45 |       25 |      50 |      50 | 6-8,11,15-17,20
  rfs.js       |   73.68 |    61.29 |      75 |   78.85 | 33,44-57,68
  serve.js     |    10.2 |        0 |       0 |   13.89 | 7-47
  store.js     |   80.75 |    58.22 |   85.71 |   89.42 | 38-43,132-136
---------------|---------|----------|---------|---------|---------------------------------------------------------------

Another question is would you like it to run a full npm test on a push? You can easily skip it you are confident that your changes dont break anything by running git push --no-verify

I dont know how this is merge conflicting, I literally copy the file over mine from master... git show remotes/origin/master:lib/wave.js > lib/wave.js

It'd be excting for code quality!

My life was hell in 2020, and I'm still dealing with the aftermath. Saw this because of noctis's comment.

The code coverage parts sound really cool (I never knew how to do those).

The linting part, as you know, I don't like. A few of the rules you picked out tho I'm in favor of, but given how much hell, stress, depression, etc. I'm recovering from, and impossible deadlines in multiple areas of my life, this is a "let's try it after mymore basic goals have been completed/finished."

Is there a way to get the code coverage without linting? Thanks!

@amark I feel like we are in a similar boat. Been overworked since the last time we chatted in Discord, which was when I created this.

As requested I broke out the code coverage, and editor config files, into separate PRs that the maintainers can comment and amend. 👍🏻

That being said I put quite alot of time into getting an eslint config that actually works with this codebase, so I hope that you will reconsider. Auto formatting and the errors in the IDE are a massive productivity boost, and to be honest quite integral to my dev workflow.

Sorry to hear that :( as well.

Been gone at bunch of conferences this last month, sorry for the delay.

Thanks for splitting things up. Which / how / teach me the parts on the code coverage?