Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 88 additions & 34 deletions .github/scripts/check_formalities.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@ MAX_SUBJECT_LEN_HARD=60
MAX_SUBJECT_LEN_SOFT=50
MAX_BODY_LINE_LEN=75

WEBLATE_EMAIL="<hosted@weblate.org>"
DEPENDABOT_EMAIL="dependabot[bot]@users.noreply.github.com"
GITHUB_NOREPLY_EMAIL='@users.noreply.github.com'
WEBLATE_EMAIL='<hosted@weblate.org>'

EMOJI_WARN=':large_orange_diamond:'
EMOJI_FAIL=':x:'

RET=0

REPO_PATH=${1:+-C "$1"}
# shellcheck disable=SC2206
REPO_PATH=($REPO_PATH)

if [ -f 'workflow_context/.github/scripts/ci_helpers.sh' ]; then
source workflow_context/.github/scripts/ci_helpers.sh
else
Expand Down Expand Up @@ -65,21 +71,34 @@ is_stable_branch() {
[ "$1" != "main" ] && [ "$1" != "master" ]
}

is_dependabot() {
echo "$1" | grep -iqF "$DEPENDABOT_EMAIL"
}

is_weblate() {
echo "$1" | grep -iqF "$WEBLATE_EMAIL"
}

exclude_dependabot() {
[ "$EXCLUDE_DEPENDABOT" = 'true' ]
}

exclude_weblate() {
[ "$EXCLUDE_WEBLATE" = 'true' ]
}

check_name() {
local type="$1"
local name="$2"
local email="$3"

if exclude_dependabot && is_dependabot "$email"; then
status_warn "$type email exception: authored by dependabot"
elif exclude_weblate && is_weblate "$email"; then
status_warn "$type email exception: authored by Weblate"
# Pattern \S\+\s\+\S\+ matches >= 2 names i.e. 3 and more e.g. "John Von
# Doe" also match
if echo "$name" | grep -q '\S\+\s\+\S\+'; then
elif echo "$name" | grep -q '\S\+\s\+\S\+'; then
status_pass "$type name ($name) seems OK"
# Pattern \S\+ matches single names, typical of nicknames or handles
elif echo "$name" | grep -q '\S\+'; then
Expand All @@ -92,14 +111,19 @@ check_name() {
fi
}

check_author_email() {
local email="$1"

if echo "$email" | grep -qF "@users.noreply.github.com"; then
output_fail 'Author email cannot be a GitHub noreply email'
check_email() {
local type="$1"
local email="$2"

if exclude_dependabot && is_dependabot "$email"; then
status_warn "$type email exception: authored by dependabot"
elif exclude_weblate && is_weblate "$email"; then
status_warn "$type email exception: authored by Weblate"
elif echo "$email" | grep -qF "$GITHUB_NOREPLY_EMAIL"; then
output_fail "$type email cannot be a GitHub noreply email"
RET=1
else
status_pass 'Author email is not a GitHub noreply email'
status_pass "$type email is not a GitHub noreply email"
fi
}

Expand All @@ -108,7 +132,9 @@ check_subject() {
local author_email="$2"

# Check subject format
if exclude_weblate && echo "$subject" | grep -iq -e '^Translated using Weblate.*' -e '^Added translation using Weblate.*'; then
if exclude_dependabot && is_dependabot "$author_email"; then
status_warn 'Commit subject line exception: authored by dependabot'
elif exclude_weblate && is_weblate "$author_email"; then
status_warn 'Commit subject line exception: authored by Weblate'
elif echo "$subject" | grep -qE -e '^([0-9A-Za-z,+/._-]+: )+[a-z]' -e '^Revert '; then
status_pass 'Commit subject line format seems OK'
Expand All @@ -130,9 +156,12 @@ check_subject() {
RET=1
fi

if exclude_weblate && is_weblate "$author_email"; then
# Don't append to the workflow output, since this is more of an internal
# warning.
# Don't append to the workflow output, since these are more of internal
# warnings.
if exclude_dependabot && is_dependabot "$author_email"; then
status_warn 'Commit subject line length exception: authored by dependabot'
return
elif exclude_weblate && is_weblate "$author_email"; then
status_warn 'Commit subject line length exception: authored by Weblate'
return
fi
Expand All @@ -159,7 +188,7 @@ check_body() {
local author_email="$3"

# Check body line lengths
if ! exclude_weblate || ! is_weblate "$author_email"; then
if ! { exclude_weblate && is_weblate "$author_email"; } && ! { exclude_dependabot && is_dependabot "$author_email"; }; then
body_line_too_long=0
line_num=0
while IFS= read -r line; do
Expand All @@ -174,24 +203,36 @@ check_body() {
if [ "$body_line_too_long" = 0 ]; then
status_pass "Commit body lines are $MAX_BODY_LINE_LEN characters or less"
fi
else
if exclude_dependabot && is_dependabot "$author_email"; then
status_warn 'Commit body line length exception: authored by dependabot'
elif exclude_weblate && is_weblate "$author_email"; then
status_warn 'Commit body line length exception: authored by Weblate'
fi
fi

if echo "$body" | grep -qF "$sob"; then
status_pass '`Signed-off-by` matches author'

# Don't append to the workflow output, since these are more of internal
# warnings.
elif exclude_dependabot && is_dependabot "$author_email"; then
status_warn '`Signed-off-by` exception: authored by dependabot'
elif exclude_weblate && is_weblate "$author_email"; then
# Don't append to the workflow output, since this is more of an internal
# warning.
status_warn '`Signed-off-by` exception: authored by Weblate'

else
output_fail "\`Signed-off-by\` is missing or doesn't match author (should be \`$sob\`)"
RET=1
fi

if echo "$body" | grep -qF "@users.noreply.github.com"; then
output_fail '`Signed-off-by` email cannot be a GitHub noreply email'
RET=1
else
status_pass '`Signed-off-by` email is not a GitHub noreply email'
if ! ( exclude_dependabot && is_dependabot "$author_email" ) && ! ( exclude_weblate && is_weblate "$author_email" ); then
if echo "$body" | grep -qF "$GITHUB_NOREPLY_EMAIL"; then
output_fail '`Signed-off-by` email cannot be a GitHub noreply email'
RET=1
else
status_pass '`Signed-off-by` email is not a GitHub noreply email'
fi
fi

if echo "$body" | grep -v "Signed-off-by:" | grep -qv '^[[:space:]]*$'; then
Expand Down Expand Up @@ -221,39 +262,52 @@ main() {
# Initialize GitHub actions output
output 'content<<EOF'

cat <<-EOF
Something broken? Consider providing feedback:
https://github.com/openwrt/actions-shared-workflows/issues

EOF

if exclude_dependabot; then
warn 'dependabot exceptions are enabled'
else
echo 'dependabot exceptions are disabled'
fi

if exclude_weblate; then
warn "Weblate exceptions are enabled"
warn 'Weblate exceptions are enabled'
else
echo "Weblate exceptions are disabled"
echo 'Weblate exceptions are disabled'
fi
echo

for commit in $(git rev-list HEAD ^origin/"$BRANCH"); do
for commit in $(git "${REPO_PATH[@]}" rev-list HEAD ^origin/"$BRANCH"); do
HEADER_SET=0
COMMIT="$commit"

info "=== Checking commit '$commit'"
if git show --format='%P' -s "$commit" | grep -qF ' '; then
if git "${REPO_PATH[@]}" show --format='%P' -s "$commit" | grep -qF ' '; then
output_fail 'Pull request should not include merge commits'
RET=1
fi

author_name="$(git show -s --format=%aN "$commit")"
committer_name="$(git show -s --format=%cN "$commit")"
check_name "Author" "$author_name"
check_name "Committer" "$committer_name"

author_email="$(git show -s --format='<%aE>' "$commit")"
check_author_email "$author_email"
author_name="$(git "${REPO_PATH[@]}" show -s --format=%aN "$commit")"
committer_name="$(git "${REPO_PATH[@]}" show -s --format=%cN "$commit")"
author_email="$(git "${REPO_PATH[@]}" show -s --format='<%aE>' "$commit")"
committer_email="$(git "${REPO_PATH[@]}" show -s --format='<%cE>' "$commit")"
check_name 'Author' "$author_name" "$author_email"
check_email 'Author' "$author_email"
check_name 'Committer' "$committer_name" "$committer_email"
check_email 'Committer' "$committer_email"

subject="$(git show -s --format=%s "$commit")"
subject="$(git "${REPO_PATH[@]}" show -s --format=%s "$commit")"
echo
info 'Checking subject:'
echo "$subject"
check_subject "$subject" "$author_email"

body="$(git show -s --format=%b "$commit")"
sob="$(git show -s --format='Signed-off-by: %aN <%aE>' "$commit")"
body="$(git "${REPO_PATH[@]}" show -s --format=%b "$commit")"
sob="$(git "${REPO_PATH[@]}" show -s --format='Signed-off-by: %aN <%aE>' "$commit")"
echo
info 'Checking body:'
echo "$body"
Expand Down
14 changes: 12 additions & 2 deletions .github/scripts/process_formalities.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const GET_COMMENTS_QUERY = `query($owner: String!, $repo: String!, $issueNumber:
}
}`;

// BUG: Classifiers are broken and they do nothing, but they must be set.
// https://github.com/orgs/community/discussions/19865
const MINIMIZE_COMMENT_MUTATION = `
mutation($id: ID!) {
minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) {
Expand All @@ -35,7 +37,8 @@ const SUMMARY_HEADER=`
>
> Some formality checks failed.
>
> Consider (re)reading [submissions guidelines](https://openwrt.org/submitting-patches#submission_guidelines).
> Consider (re)reading [submissions guidelines](
https://openwrt.org/submitting-patches#submission_guidelines).

<details>
<summary>Failed checks</summary>
Expand All @@ -52,7 +55,13 @@ const NO_MODIFY=`
>
> PR has _Allow edits and access to secrets by maintainers_ disabled. Consider allowing edits to simplify review.
>
> [More info](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
> [More info](
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
`;

const FEEDBACK=`
Something broken? Consider [providing feedback](
https://github.com/openwrt/actions-shared-workflows/issues).
`;

async function hideOldSummaries({ github, owner, repo, issueNumber }) {
Expand Down Expand Up @@ -86,6 +95,7 @@ function getCommentMessage({ context, jobId, noModify, summary }) {
return `
${summary.length > 0 ? getSummaryMessage({ context, jobId, summary }) : ''}
${noModify ? NO_MODIFY : ''}
${FEEDBACK}
${COMMENT_LOOKUP}
`;
}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/formal-this-repo.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: Test Formalities

on:
pull_request:
pull_request_target:

permissions:
contents: read
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/formal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ name: Test Formalities
on:
workflow_call:
inputs:
exclude_dependabot:
description: 'Exclude commits authored by dependabot from some checks'
default: true
required: false
type: boolean
exclude_weblate:
description: 'Exclude commits authored by Weblate from some checks'
required: false
Expand Down Expand Up @@ -47,6 +52,7 @@ jobs:
run: workflow_context/.github/scripts/check_formalities.sh
env:
BRANCH: ${{ github.base_ref }}
EXCLUDE_DEPENDABOT: ${{ inputs.exclude_dependabot }}
EXCLUDE_WEBLATE: ${{ inputs.exclude_weblate }}

- name: Process GitHub formality check results
Expand Down