Add the editor configuration and a formatting leg with its own name - #68
Merged
Merged
Conversation
Closes #23. The tree holds HTML, YAML, JSON, Go and prose, and nothing decided whitespace in any of them. Review time goes on it and diffs carry changes nobody made. .editorconfig is the request, and it records what the tree already is rather than a preference imposed on it: LF, a final newline, no trailing whitespace, four spaces except where an ecosystem uses two and except Go, which follows gofmt rather than arguing with it. internal/format is the refusal, and it is Go for the reason decisions/means.md gives: a rule in a workflow step has no suite and cannot be run against a planted input, and every property here is one function reading bytes and returning a verdict. It judges three things gofmt does not reach, over the files git tracks rather than whatever the working directory holds, so a scratch file nobody committed cannot red the leg. The Formatting workflow is the leg, with its own name, so a red says formatting rather than something in the build and so it can be required on its own when #48 arrives. The line-ending trap is the part worth getting right rather than discovering later. internal/format normalises CRLF before judging, and a test compares the verdicts on both spellings of the same content, so it cannot become an operating-system check by accident. gofmt does not normalise and lists a Go file whose working copy has CRLF, which is why .gitattributes widens from the JSON files to the whole tree with eol=lf: a fresh clone is LF in the working copy whatever core.autocrlf says locally, so gofmt agrees with itself on every platform. Measured rather than assumed, on a working copy that had CRLF and a fresh checkout that did not. CONTRIBUTING.md names the format command and says which half handles line endings which way, including what to do when gofmt lists a file nobody touched. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com>
This branch was written before #18 landed and said so: it added .github/workflows/format.yml with its own gofmt step and its own `go test ./internal/format` step, and its comment promised to call the single entry point once one existed. One exists on main now, and the promise was still unkept, so merging as it stood would have put a second gofmt run on every pull request under a different name and left the whitespace rules in a job the gate's own suite cannot see. What was wrong is the shape rather than the rules: internal/format, .editorconfig and the .gitattributes widening are unchanged. The whitespace half is now the `editorconfig` leg of `go run . gate`, with a job in gate.yml like every other leg, and format.yml is gone. This prevents two failures. A leg living outside the entry point drifts from what a contributor runs before pushing, which is the thing #18 exists against. And a check outside gate.yml is invisible to TestWorkflowDeclaresOneJobPerLeg, so deleting its job would have been green. Found by reading the branch against main before merging. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #23.
Two halves, as the issue asks.
.editorconfigso the common cases are rightbefore anybody runs anything, and a check in the gate so the rule is refused
rather than requested. The check is its own leg with its own name.
What decides what
gofmt decides Go. It ships with the toolchain,
decisions/means.mdnames it,and re-implementing any part of it here would be a fight with the formatter the
compiler agrees with.
internal/formatdecides everything gofmt does not reach, which is most of thetree: the HTML, the JSON, the workflow YAML and the prose. Three properties, all
of them ones the tree already satisfies rather than a preference imposed on it: a
final newline, no trailing whitespace, and no tab indent outside Go.
Run at 5995c3e. Four of those types have no formatter that does not bring a
runtime with it, which is why the answer is a check plus an editor
configuration rather than a formatter.
It is Go for the reason
decisions/means.mdgives. Each property is a functionreading bytes and returning a verdict, which means it can be run against a
planted input; the same rule as a shell fragment in a workflow step has no suite
and can only be shown to bite by breaking the tree on purpose. No dependency is
added, so
go.modis untouched and there is still nogo.sum.It reads the files git tracks rather than whatever the working directory holds,
so an untracked scratch file in somebody's clone cannot red the leg.
The correction this branch carries
The first version of this change put its own workflow file in the tree,
.github/workflows/format.yml, with a gofmt step and a step running theinternal/formatsuite. Its comment said what it was waiting for: the singleentry point of #18, and calling that instead once it existed. #18 has since
landed on main as
go run . gate, so the branch was carrying a promise it couldnow keep, and merging it unkept would have added a second gofmt run to every
pull request under a name the gate's own suite does not read.
The rules did not move.
internal/format,.editorconfigand the.gitattributeswidening are the same bytes as before. What moved is where thewhitespace half runs: it is the
editorconfigleg of the entry point, with ajob in
gate.ymllike every other leg, andformat.ymlis deleted.Two legs rather than one widened leg, because a
Legruns one command and thetwo halves fail for different reasons: one is the toolchain's formatter, the
other is three properties
.editorconfigstates. A red says which.The line-ending half, which is the part worth getting right first
The issue names it: a formatting gate that reds only on one operating system
pushes contributors away for a reason they cannot see. The two halves get there
differently and only one of them is self-sufficient.
internal/formatnormalises CRLF before judging, and a test compares theverdicts on both spellings of the same content so it cannot quietly become an
operating-system check:
gofmt does not normalise, and this was measured rather than assumed. On a
working copy left with CRLF it lists files whose content is correct:
What keeps that from being a Windows-only red is
.gitattributes, which widensfrom the JSON files to the whole tree with
eol=lf. A checkout under the newattributes is LF in the working copy as well as in the object store, whatever
core.autocrlfsays locally, and gofmt then agrees with itself:All run at 5995c3e on Windows with
core.autocrlftrue, which is the checkoutthat would have shown the problem. Widening
.gitattributesrenormalisednothing, because every tracked blob was already stored with LF. Nothing under
.gitattributesorinternal/formathas changed since, so those outputs arestill about the bytes this branch lands. The runs below are at the head.
The leg refusing something
Trailing whitespace planted on the first line of
README.md, then the leg:The job deleted from
gate.yml, which is what makes the leg visible to thegate's own suite rather than to nobody:
Both run at a9b41a5 against a working copy restored afterwards; neither break is
in the diff. Each of the three whitespace rules also has a unit leg that plants
it in memory, so a rule cannot pass because the tree happens not to exercise it.
The whole gate, clean
Run at a9b41a5.
What this does not do
It does not format anything. There is no formatter for HTML, YAML or Markdown
here, and adding one means adding a runtime
decisions/means.mdruled out. Thecheck refuses; the editor configuration is what fixes it before you get there.
It does not judge a lone carriage return or an indent width. The first is not in
the tree and the second cannot be judged without parsing each language, which is
a formatter and not a whitespace rule.
Requiring the leg on
mainis #48 and is not done here.No second person has read this change. What stands in place of a review is the
evidence above: the gate passing, two planted breaks refused, the line-ending
property measured in both directions, each with the command that produced it.