martinthomson / i-d-template

A template for IETF internet draft git repositories
Other
204 stars 180 forks source link

Lint output #391

Closed mnot closed 4 weeks ago

mnot commented 1 year ago

Currently, the github action markdown summary shows the output of the build process; e.g., https://github.com/httpwg/http-extensions/actions/runs/5284538905

Is it possible for an extension linter to also write to this?

(and, is there any documentation to help people writing things like linters?)

martinthomson commented 1 year ago

Yes, it is possible. But I don't currently have the time to document it fully.

There is a script wrapper that produces output to a predetermined file. You can see this here: https://github.com/martinthomson/i-d-template/blob/5ccd7fb189a4030283e82a1cc09bafe80c09d7ae/main.mk#L111

You can copy this pattern in your linting if you like.

The arguments to the script are the name of the output file (the file extension is stripped off by the script to keep your rule lean), an optional -s flag followed by a stage name (any text, you would choose the name of the linting tool probably; the default is the name of the linting tool executable), and the remainder being the command to wrap.

The trace script records output to $TRACE_FILE. If that environment variable isn't set, then the command is executed directly. The trace file has multiple lines for each file and stage. All lines start with two space separated items: file and stage. The first line for a given file and stage contains the status (0 = success, not 0 = failure). Subsequent lines contain any output you want in the summary (by default, I take 16 lines from the output; it was 10, but kramdown-rfc dumps stack and it tends to hide the useful content 12 or so lines back).

mnot commented 1 year ago

Thanks. What is "output file" in the context of a linter?

martinthomson commented 1 year ago

"output file" might be "linted file". This is just a grouping convenience for the summary report; I use the draft name. In most cases, you want "draft-name-wg-protocol", so that the summary will show errors associated with that draft (and not a ✅).

If you are linting multiple drafts, then you might need to build the reporting for each into the linting tool. I'd avoid that.

You currently have:

http-lint: $(drafts_xml) $(DEPS_FILES)
    $(rfc-http-validate) -q -m sf.json $(filter-out $(DEPS_FILES),$^)

It might make sense to do something like:

http-lint: $(add-suffix .http-lint.txt,$(add-prefix .,$(drafts)))
.%.http-lint.txt: %.xml $(DEPS_FILES)
    $(trace) $< -s http-lint $(rfc-http-validate) -q -m sf.json $<
    @touch $@
mnot commented 1 year ago

OK, trying that - thx

mnot commented 1 year ago

This is where I ended up:

clean::
    -rm -f .*.http-lint.txt

lint:: http-lint

rfc-http-validate ?= rfc-http-validate
.SECONDARY: $(drafts_xml)
.PHONY: http-lint
http-lint: http-lint-install $(addsuffix .http-lint.txt,$(addprefix .,$(drafts)))
.PHONY: .%.http-lint.txt
.%.http-lint.txt: %.xml $(DEPS_FILES)
    $(trace) $< -s http-lint $(rfc-http-validate) -q -m sf.json $<
    @touch $@

.PHONY: http-lint-install
http-lint-install:
    @hash rfc-http-validate 2>/dev/null || pip3 install rfc-http-validate
martinthomson commented 1 year ago

You can add rfc-http-validate to a fresh requirements.txt and avoid that last little bit. Then the $(DEPS_FILES) thing will sort it out installation for you (and do periodic updates).

martinthomson commented 4 weeks ago

It looks like this one was resolved happily. We will continue to iterate, I'm sure.