Conversation
There was a problem hiding this comment.
This checker is explicitly trying to find these invalid words in these cases, so I don't think we want them changed. Not 100% sure how to say "this is acceptable" for the soundness checking in a multi-line Swift string here though
|
|
||
| DispatchQueue.global().asyncAfter(deadline: .now() + 1) { | ||
| kill(getpid(), SIGABRT) | ||
| kill(getpid(), SIGABRT) # ignore-unacceptable-language |
| /// Runs `signal-test-app` and confirms that it exits with code 99. | ||
| /// | ||
| /// `signal-test-app` sends a kill signal to itself and uses ``Signal`` to intercept that signal | ||
| /// `signal-test-app` sends a terminate signal to itself and uses ``Signal`` to intercept that signal |
There was a problem hiding this comment.
"kill" and "terminate" are terms of art in the case of signals to stop process execution.
In this case it might be better to spell out the specific signal being sent to avoid any other interpretation
| /// `signal-test-app` sends a terminate signal to itself and uses ``Signal`` to intercept that signal | |
| /// `signal-test-app` sends a `SIGABRT` signal to itself and uses ``Signal`` to intercept that signal |
There was a problem hiding this comment.
These checks feel unrelated to the PRs main purpose and could IMO be added later.
compnerd
force-pushed
the
shahmishal/github-actions-support
branch
from
6d8fafe to
fb0fa80
Compare
|
It's a bit pointless to request additional reviews when the feedback so far hasn't been addressed yet. |
d-ronnqvist
left a comment
d-ronnqvist
left a comment
There was a problem hiding this comment.
There's code that doesn't compile. After that is fixed; DocC's own inclusive language test fail because all the non-inclusive language has been removed from that test content.
Also, the whitespace checking script seems unnecessary and unrelated to having supporting GitHub actions. Please move that to a separate PR so that it can be discussed on its own merits and not hold up this PR.
3 participants
No description provided.