swift-metrics
swift-metrics copied to clipboard
apple
Add documentation and commit hook for SwiftFormat.
Motivation:
We enforce the use of SwiftFormat in our sanity.sh script,
but do not mention that in our documentation. We should point
out the usage in our Contributing guidelines, and also a helper
to automatically run it as a git commit.
Modifications:
Added details about SwiftFormat, a commit hook, and also the .swift-version to make sure SwiftFormat is able to run based on the base version of Swift we want to support.
Result:
Contributors will be aware of the formatting requirements of the repo.
and also the .swift-version to make sure SwiftFormat is able to run based on the base version of Swift we want to support.
Does SwiftFormat use that? Because it also causes swift-env to use such version... In general I don't think it's so good to commit such files, as it may behave differently for some people than others (if one has swift-env or not) -- even if one uses them locally (that's fine)
Failure is missing license headers:
* bash... missing headers in file './scripts/pre-commit.sh'!
Can you please add @Yasumoto ?
Failure is missing license headers:
* bash... missing headers in file './scripts/pre-commit.sh'!Can you please add @Yasumoto ?
Ah, it’s a little more nuanced:
--- /dev/fd/63 2019-10-05 00:36:51.264075715 +0000
+++ /tmp/.swift-metrics-sanity_TbyWkM 2019-10-05 00:36:51.055076377 +0000
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
##===----------------------------------------------------------------------===##
In general I recommend we stick with POSIX shell when possible vs. bash since it’s much simpler; but I’m open to pushback!
Maybe instead of using this script as pre-commit hook, connect it to the already existing sanity.sh? It includes swift-format & some other things (headers & linux tests)
In general I recommend we stick with POSIX shell when possible vs. bash since it’s much simpler; but I’m open to pushback!
@Yasumoto no objections, but it means amending https://github.com/apple/swift-metrics/blob/master/scripts/sanity.sh#L91, to support both
Maybe instead of using this script as pre-commit hook, connect it to the already existing sanity.sh? It includes swift-format & some other things (headers & linux tests)
+1
I ran ShellCheck on the script as well, I considered but backed away from adding it to the sanity.sh script since I didn't want to change too much at once. However, definitely happy to do so if that's desired!
Huh sorry we dropped the ball on this.
It seems we might be trying to pull off a swiftpm plugin for soundness checks so I'll close this for now -- also since it'll need a rebase.