lint: establish jessie as a coding style norm

[warner]

We talked this morning about establishing a way to specify which packages (and which files within those packages) are supposed to conform to the Jessie subset, and to set up eslint rules to check that.

One option is a per-file header comment (perhaps 'use jessie';, analogous to 'use strict';). Another is a config file in the package (packages/ERTP/.eslint-jessie) which signals that the whole package should conform.

We need to build some tooling to run eslint in a uniform way against everything, and then get CI running it on every PR.

@katelynsills @erights @michaelfig had some thoughts on this

[katelynsills]

So far in some packages we have optional jessie linting in package.json scripts:

    "lint-fix": "eslint --fix '**/*.{js,jsx}'",
    "lint-check": "eslint '**/*.{js,jsx}'",
    "lint-fix-jessie": "eslint -c '.eslintrc-jessie.js' --fix '**/*.{js,jsx}'",
    "lint-check-jessie": "eslint -c '.eslintrc-jessie.js' '**/*.{js,jsx}'"

To be able to run lint-fix-jessie only on some files immediately, we could use the configuration settings to disable rules for a group of files

[erights]

I think what we want longer term is a 'use jessie' directive, parallel to 'use strict', because it fits so well into the language. Mozilla's asmJS system recognized a 'use asm' directive. If we agree that this is what we want long term, I suggest the following short term kludge:

The 'use jessie' directive must only appear on the first line of the file. We write a little program that, given a list of file names as arguments, lists on standard-out the subset of that list that contains the string 'use jessie' in their first line. Then we can wrap the wild cards in the above package.json scripts with a call to this util, giving back the jessie subset of the wildcard match.

[michaelfig]

I have a sample .eslintrc.js file that automatically detects the 'use jessie'; token and uses an override to choose the jessie config.

https://github.com/Agoric/eslint-config-jessie/pull/16

[dckc]

I was going to mark this closed as of when we started enforcing jessie rules on issuers.js (Apr 21 fd1c24a84584) but when it got split into issuerKit.js and paymentLeder.js (6c80a619fcc1) the @jessie-check declaration didn't get into paymentLedger.js and I don't know whether that was by design or not.

I suppose it will take more than just one file to establish a norm that we're enforcing.

cc @katelynsills

[katelynsills]

I can look into adding @jessie-check back into ERTP. That sounds like a good idea, especially pre-audit.

[Tartuffo]

@dckc You said you were going to mark as closed, but did not.

[dckc]

I'm not sure what you're referring to.

[Tartuffo]

@dckc Can we close this ticket?

[dckc]

No, I don't see much change since July 22 when I wrote:

I suppose it will take more than just one file to establish a norm that we're enforcing.

@erights is probably the main customer here. What do you think, MarkM?

[erights]

@erights is probably the main customer here. What do you think, MarkM?

I don't think three will establish the norm either, and issuerKit.js is rather central. I'm closing this, but we need to remember our shared software quality goal is to have much more code in checked Jessie.

[dckc]

I use the issues list as (the reliable part of) my memory. I take closing this as a signal that in fact we do not need to remember, though it would be nice.

[erights]

Makes sense. Reopen and change title? Or leave closed and open a new issue? Or a wiki page or something? Your choice ;)

[erights]

thanks