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

grab bag of dev goodies, gha, lint, dep versions #607

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

msimerson
Copy link

@msimerson msimerson commented Mar 18, 2022

  • fixes stagnant / broken Travis CI
  • adds CI support for Github Workflows/Actions
    • lint and docs are separate tests (more efficient, narrows down why a PR is failing)
      • docs has to be separate, b/c npm install rewrites package-lock.json differently between node 14 and 16
    • testing in node versions 12, 14, 16
    • windows CI support can be added to ci.yml.strategy.matrix.os, but there are os.EOL and path.join updates needed.
  • adds inoffensive linting via eslint
    • replaces Code formatter #314
    • I've configured it so that it shows some current code issues but doesn't require making any changes
    • I like the idea of prettier, but my aesthetic sensibilities run closer to @kach, I wouldn't have merged that PR either
    • adds script targets: lint and lintfix
  • dependency version checker
  • dependency updates required for a successful CI build:

@msimerson
Copy link
Author

msimerson commented Mar 18, 2022

dependency version checks

➜  nearley git:(github-workflows) npm run versions

> nearley@2.20.1 versions
> npx dependency-version-checker check

Performing dependency updates check for project: /Users/matt/git/nearley/package.json.
Check will be performed for dependencies matching this regex: /.*/.

You could update 12 dependency(/-ies).

Dependency         Type  Current Version  Latest Minor  Latest Major  
=================  ====  ===============  ============  ============  
commander          Prod  2.19.0           2.20.3        9.0.0         
moo                Prod  0.5.0            0.5.1         -             
railroad-diagrams  Prod  Error caught: Could not parse as JSON: 1.0.0
  
randexp            Prod  0.4.6            0.5.3         -             
@types/moo         Dev   0.3.0            0.5.5         -             
@types/node        Dev   7.0.27           7.10.14       17.0.21       
babel-cli          Dev   6.18.0           6.26.0        -             
babel-preset-env   Dev   1.6.0            1.7.0         -             
benchr             Dev   3.2.0            3.4.0         4.3.0         
coffeescript       Dev   2.6.1            -             99.999.99999  
doctoc             Dev   1.3.0            1.4.0         2.1.0         
expect             Dev   1.20.2           -             27.5.1        
microtime          Dev   3.0.0            -             -             
mocha              Dev   9.2.2            -             -             
typescript         Dev   3.9.7            3.9.10        4.6.2   

I purposefully avoided updating dependency versions in this PR.

@msimerson
Copy link
Author

CI support

Over on my fork of nearley, you can click the Actions tab and see the status of CI tests. I think it's pretty self explanatory but I'm happy to answer any questions.

@msimerson
Copy link
Author

Lint support

This is the current lint output the recommended rules and a few currently defined additions I made, mostly to reduce the severity of lint errors, which would require making more code change, which I have tried very hard to avoid.

One example is that it appears that older code in the repo has semicolons. However, there's far more semicolons than not, so I set the rule to require them and only warn. It'd be easy to remove them all by changing the rule and running npm run lintfix, but I think changes like that belong in a separate PR.

➜  nearley git:(github-workflows) npm run lint

> nearley@2.20.1 lint
> npx eslint bin/*.js lib/*.js


/Users/matt/git/nearley/bin/nearley-railroad.js
   7:255  warning  Missing semicolon  semi
   8:18   warning  Missing semicolon  semi
  63:21   warning  Unreachable code   no-unreachable
  66:21   warning  Unreachable code   no-unreachable
  69:21   warning  Unreachable code   no-unreachable

/Users/matt/git/nearley/bin/nearley-test.js
   4:75  warning  Trailing spaces not allowed                       no-trailing-spaces
  28:43  warning  Missing semicolon                                 semi
  39:13  warning  'stateNumber' is assigned a value but never used  no-unused-vars
  42:11  warning  Missing semicolon                                 semi
  43:7   warning  Missing semicolon                                 semi
  45:2   warning  Missing semicolon                                 semi
  50:2   warning  Missing semicolon                                 semi

/Users/matt/git/nearley/bin/nearley-unparse.js
  4:5  warning  'nearley' is assigned a value but never used  no-unused-vars
  6:5  warning  'randexp' is assigned a value but never used  no-unused-vars

/Users/matt/git/nearley/lib/compile.js
  71:76  warning  Missing semicolon  semi

/Users/matt/git/nearley/lib/generate.js
   11:29   warning  Missing semicolon  semi
  106:83   warning  Missing semicolon  semi
  141:6    warning  Missing semicolon  semi
  193:104  warning  Missing semicolon  semi
  193:116  warning  Missing semicolon  semi

/Users/matt/git/nearley/lib/lint.js
  5:2  warning  Missing semicolon  semi

/Users/matt/git/nearley/lib/nearley-language-bootstrapped.js
    7:22   warning  Missing semicolon                 semi
   11:19   warning  Missing semicolon                 semi
   13:48   warning  Missing semicolon                 semi
   15:17   warning  Missing semicolon                 semi
   18:25   warning  Missing semicolon                 semi
   21:15   warning  Unnecessary escape character: \#  no-useless-escape
   22:26   warning  Unnecessary escape character: \>  no-useless-escape
   24:19   warning  Unnecessary escape character: \%  no-useless-escape
   24:29   warning  Unnecessary escape character: \%  no-useless-escape
   24:37   warning  Unnecessary escape character: \%  no-useless-escape
   28:23   warning  Unnecessary escape character: \?  no-useless-escape
   28:25   warning  Unnecessary escape character: \+  no-useless-escape
   45:4    warning  Missing semicolon                 semi
   59:3    warning  Missing semicolon                 semi
   78:76   warning  'ws' is not defined               no-undef
   83:87   warning  'arrow' is not defined            no-undef
   84:143  warning  'arrow' is not defined            no-undef
   84:245  warning  Missing semicolon                 semi
   87:148  warning  Missing semicolon                 semi
   88:148  warning  Missing semicolon                 semi
   98:115  warning  Missing semicolon                 semi
   99:176  warning  Missing semicolon                 semi
  103:115  warning  Missing semicolon                 semi
  112:72   warning  'word' is not defined             no-undef
  113:78   warning  'string' is not defined           no-undef
  114:82   warning  'btstring' is not defined         no-undef
  115:87   warning  'charclass' is not defined        no-undef
  116:66   warning  'js' is not defined               no-undef
  120:66   warning  'ws' is not defined               no-undef
  121:73   warning  'ws' is not defined               no-undef
  123:89   warning  'comment' is not defined          no-undef
  126:2    warning  Missing semicolon                 semi
  130:4    warning  'window' is not defined           no-undef

/Users/matt/git/nearley/lib/nearley.js
   25:6   warning  Missing semicolon                                                          semi
  111:21  warning  'exp' is already defined                                                   no-redeclare
  121:35  warning  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  123:34  warning  'i' is already defined                                                     no-redeclare
  134:6   warning  Missing semicolon                                                          semi
  145:6   warning  Missing semicolon                                                          semi
  150:6   warning  Missing semicolon                                                          semi
  158:25  warning  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  172:13  warning  'rules' is already defined                                                 no-redeclare
  176:6   warning  Missing semicolon                                                          semi
  188:6   warning  Missing semicolon                                                          semi
  199:6   warning  Missing semicolon                                                          semi
  205:8   warning  Missing semicolon                                                          semi
  206:6   warning  Missing semicolon                                                          semi
  216:48  warning  Trailing spaces not allowed                                                no-trailing-spaces
  240:6   warning  Missing semicolon                                                          semi
  245:17  warning  'options' is already defined                                               no-redeclare
  247:17  warning  'grammar' is already defined                                               no-redeclare
  266:13  warning  'table' is assigned a value but never used                                 no-unused-vars
  284:16  warning  Unexpected constant condition                                              no-constant-condition
  309:17  warning  'nextColumn' is already defined                                            no-redeclare
  343:21  warning  'err' is already defined                                                   no-redeclare
  351:47  warning  Missing semicolon                                                          semi
  357:41  warning  Missing semicolon                                                          semi
  422:6   warning  Missing semicolon                                                          semi
  423:1   warning  Trailing spaces not allowed                                                no-trailing-spaces
  497:76  warning  Missing semicolon                                                          semi
  508:55  warning  Missing semicolon                                                          semi

/Users/matt/git/nearley/lib/unparse.js
  128:17  warning  'grammar' is already defined  no-redeclare

✖ 83 problems (0 errors, 83 warnings)
  0 errors and 46 warnings potentially fixable with the `--fix` option.

@kach
Copy link
Owner

kach commented Mar 18, 2022

Thanks for this. Quick question — in this PR do you also intend to fix the issues reported by the linter? I guess the linter is only helpful if we do that, but I don't think I'll have time to go through the issues myself in the near future.

@msimerson
Copy link
Author

I'd be happy to fix the lint issues, but prefer doing each issue in a separate PR.

Example: inconsistent semicolon usage. They used to be kinda standard in JS community but now they're mostly dispensed with. What is THIS projects preference? That issue is a PR, with possible conversation all of its own.

@msimerson
Copy link
Author

➜  nearley git:(github-workflows) npm run benchmark

> nearley@2.20.1 benchmark
> node ./node_modules/benchr/bin/benchr test/benchmark.js

• test/benchmark.js:

  • calculator: parse.

    ✔  nearley  14,969.64  ops/sec  ±0.65%  (91 runs)

  • json: parse sample1k.

    ✔  nearley  146.92  ops/sec  ±2.11%  (76 runs)

  • tosh: parse.

    ✔  nearley  2,135.25  ops/sec  ±1.55%  (91 runs)

@msimerson
Copy link
Author

ping

@kach
Copy link
Owner

kach commented Apr 8, 2022

Hi Matt - thanks for the reminder. In general changes that don't fix bugs or add vital functionality are very low priority for me — actually, they are a net risk because the project as it stands works (even if the code looks a little dated) and changes might break something unexpected for someone somewhere. I'd love to have time to review all pull requests carefully and immediately, but realistically that's hard on top of the already-more-than-full-time obligations of being a student. Unless there is a compelling urgency, I expect to get to this and other nearley-related items during the first few weeks of summer break. Thanks for your patience!

@DavidTanner DavidTanner mentioned this pull request Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't install dependencies
2 participants