Cross-posting some discussion in EIP-editor channel, at permission of @matt and @SamWilsn
lightclient — Today at 1:47 PM
i think this is more of a symptom of lots of process changes rather than something that needs to change about eipw - if we stabilize the policy of things, authors won’t have trouble making small PRs because eip is simply up to date
SamWilsn — Today at 1:49 PM
That’s a really good point. As more EIPs get updated, the number of these rule breaking EIPs goes down.
There’s a lot of work in implementing that. It’s a hell of a lot more complicated than checking the whole file.
xinbenlv — Today at 1:57 PM
I could understand if there is technical difficulty, and is there non-technical reason not to? If the only blockage is technical, we can set it as a policy: ignoring errors when you are not touching that line, but right now we can’t implement it in EIPW due to technical challenge. It will create two outcomes:
People with EIPW expertise could offer to help implementing it.
Editors or editorial contributors could offer to manually signal bypassing it when EIPW make a complaint due to this.
There’s the “always leave code better than you found it” guideline: if you change an EIP, it’s your responsibility to improve it. I’ve overridden eipw for minor changes before, and will continue to do so in the future.
Unpredictable is generally bad. If an author is working on their own EIP, and gets inconsistent feedback depending on what lines they change, they won’t know what to fix. It also shifts a lot of the formatting changes to the status changing commit, instead of providing feedback early.
Lines are a poor level of granularity for this. Most EIPs, as far as I remember, use one line per paragraph, so you won’t end up solving the issue if the edit hits a line with an error on it.
The problem of updating a line blocked by many errors has historically blocked many attempts to update EIPs, scare away many potential EIP contributors. I think it’s critical to make this policy clear and hence warrant a individual Meta EIP
I can see the argument for only checking changed lines, but agree with @SamWilsn’s point that having consistent whole-file feedback is useful.
Another approach would be to not introduce any lints that don’t already pass on the entire codebase. This would be standard software-engineering practice – the fact that the codebase fails its own CI on the master branch is what I found irritating. I’m a fan of the “Bors”/merge queue system, where changes are always integrated into master and fully tested before merging (a bit of history on this system here).
The downside is that this puts the onus on lint/policy writers to go and fix hundreds of EIPs before their policy can be merged. As a fan of a more lightweight policy, I’m tempted to say that this is an OK tradeoff. If fixing the lint can’t be automated and easily rolled out across the codebase, then maybe it shouldn’t exist. Some lints are easy to automate, e.g. the metadata ordering lint that I ran into. The URL lints are harder to fix automatically, unless URLs can be auto-converted to archive.org links or similar.
I strongly recommend running lints only on diffs. Some reasons mentioned above, like encourage people to do small changes, without requiring them to commit to full doc refactor.
Documents are not source code, that requires CI and UT, so existing document can be linted incrementally, over the.
Another reason is document history: I like to use git history to see evolution of source/document.
The current lint system breaks this, since a seamingly small change to a file turns into a huge refactor.
Yes, it would be nice to encourage editors to run a “refactor” session once in a while.
However, editing a document is currently a complex task, with forking and repeatedly running a local linter.
I wonder if there is a way to make it more interactive: first, allow fto view files online with lint marks. Then a web based online editor with integrated linter (eg based on vscode).
I think such a tool will promote linting the docs much better than the current fix-all-on-every-commit mode.