Should we lint and format code of swift-format with swift-format? #610
Comments
|
I think in an ideal setup, Since I see someone (@Iron-Ham, a GitHub staff member even!) has written an Action already (I worked on similar for PHP, linking here for the screenshot). It only runs lint, not format. The biggest missing piece I see is filtering linting to only lines changed in the PR. I've been looking at using libgit2 (add via SPM instructions) to generate a patch/diff and then only output changes whose lines are in the venn diagram of errors and changes. I'm a little out of my depth still so don't be disappointed if I don't open a PR soon. I think git_patch_from_blobs might be the correct function to use. I feel your itch. I have a nice Action on PHP projects that updates a README badge to grey with a red X when linting is failing! |
|
Yeah, I absolutely agree that we should do this; now that swift-syntax verifies its formatting with swift-format, I'd like to do the same for our own repository. I think it would be fine to just adopt something similar to what they've done in their testing script, where they run swift-format and look at the diffs afterward. I'd like to not stray too far from what the other Apple projects are doing, so that we have some internal consistency there. However, it's been difficult to tilt at smaller issues like this until we have CI support directly on the swift-format repository. Right now the only CI coverage we get is by running it through a paired PR on another repository like swift-syntax. Before we start adding additional requirements for PRs to pass successfully, I would really like it if we can get CI triggers on swift-format itself so that we don't have a needlessly complicated workflow for contributors. @shahmishal @ahoppen |
I'd suggest looking into |
|
Tracked in Apple’s issue tracker as rdar://126948263 |
I was working on a prototype for #607 and noticed a few lines with trailing spaces here and there, and thought I'd check for formatting rules. Apologies if I missed this in contributing docs, but it seems that:
.swift-formatconfiguration in particularScripts/format-diff.sh[Scripts/format-diff.sh], though, but I don't know if it's hooked up in a CI (and no GitHub CI checks are set up, seemingly?)How are we doing now?
I tried running the default configuration, and
swift-formatis a bit angry with itself:What should we do about it?
nicklockwood/SwiftFormat, butswift-syntaxusesswift-formatand indents with two spaces — I'm in the beginner league, and not sure how that will evolve long-term, but it does seem reasonable to suggest usingswift-formatfor this repo source code?https://github.com/apple/swift-syntax/blob/main/format.pyscript fromswift-syntax, and perhaps use consistent.swift-formatrules as well — their config is pretty minimal.Why bother at all?
I get an itch to commit those formatting cleanups, but as they are not related to a particular issue I'm working on, they're distracting for myself and for folks who would review my code. It's probably a good idea to clean them up in a single pass. Plus, that will make
swift-formatconsistent withswift-syntaxfor both ourselves, and community contributors like myself.TODOs
If generally adding formatting and linting to CI seems like a good idea, and if we confirm that
swift-syntaxformat.pyis a good starting point, then:Documentation/Development.mdandCONTRIBUTING.mdto mention them.I'm happy to help in any way I can — and can start with the first two items on this list.
The text was updated successfully, but these errors were encountered: