Currently some Author Pain comes from the EIPW complains about lint errors that were not introduced in that specific pull request they are working on, such as
links that were not allowed
mandates to call ERC “ERC” or call ERC “EIP”
spaces between section
order of section
In particular, sometimes new policy will be imposed on EIP, and it means many existing EIP are in the state of inconsistency.
This is particular confusing and painful which requires Authors to get up to date to EIP editorial policy.
I hereby propose a change to EIPW:
For any PR that’s not changing state or Draft/Review, only lint the line of code being changed.
Only lint full EIP when moving into Last Call or Final
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.
I hear that Editor @SamWilsn (and @Pandapip1 previously) share the stronger view which is if you touch it, you have responsibility to fix all linter errors.
I am generally pro micro contribution, therefore, I am arguing for allowing changes as long as it’s not making the EIP worse.
I don’t think this needs an EIP. eipw isn’t even part of the official process; it’s just a tool that enforces the general rules of the process as defined by us, the editors.
Just wanna officially document down this consensus, make sure there is no ambiguity, so next time when something comes up we can point people to this consensus.
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 don’t think this needs to be in a separate EIP. Agreement can be shown in this discussion. I’d like to avoid a proliferation of small meta EIPs about processes.
EIP-1: EIP Purpose and Guidelines has a style guide section, couldn’t you add any high level linting rules to this (or a separate section on linting)
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.
We do exercise editor discretion here. For example, if someone puts up a pull request fixing a spelling mistake (but doesn’t fix all the linter errors) we generally merge it.