Skip to content

Add PR platform labeling #35327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JanKrivanek wants to merge 1 commit into from
Closed

Add PR platform labeling #35327

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 61 additions & 0 deletions .github/workflows/platform-label.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Auto-label PRs with platform/android, platform/ios, platform/macos, and/or platform/windows
# based on changed file paths. Applies all matching labels.
# If no platform-specific files are detected, no labels are applied.

name: Platform Label

on:
pull_request:
types: [opened, synchronize]

@MauiBot MauiBot May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Coverage — Missing reopened trigger event

A PR closed and then reopened (common after merge conflicts are resolved or after a draft→ready transition that was previously closed) will not have its platform labels evaluated. Add reopened to ensure labels are applied:

types: [opened, synchronize, reopened]

@MauiBot MauiBot May 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Coverage — Missing reopened trigger event

A PR closed and reopened (draft→ready cycling, or revived after merge-conflict resolution) will not have its platform labels evaluated. Add reopened:

types: [opened, synchronize, reopened]


permissions:
contents: read
pull-requests: write

jobs:
label:
runs-on: ubuntu-latest
steps:
- name: Detect platform from changed files
env:
GH_TOKEN: ${{ github.token }}
run: |
PR_NUMBER="${{ github.event.pull_request.number }}"

FILES=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename' 2>/dev/null || true)

@MauiBot MauiBot May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[moderate] Safety — Context expressions interpolated directly into shell

${{ github.repository }} is interpolated inline into the shell script. GitHub's own security guidance (https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections) recommends passing context values as env vars to prevent script injection:

env:
  GH_TOKEN: ${{ github.token }}
  GH_REPO: ${{ github.repository }}
  PR_NUMBER: ${{ github.event.pull_request.number }}

Then reference $GH_REPO and $PR_NUMBER in the shell. While dotnet/maui is safe, github.repository for forks could theoretically contain shell metacharacters.

@MauiBot MauiBot May 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[moderate] Safety — Context expressions interpolated directly into shell

${{ github.repository }} is interpolated inline into the run: script. GitHub's script-injection guidance recommends passing context values as env: vars:

env:
  GH_TOKEN: ${{ github.token }}
  GH_REPO: ${{ github.repository }}
  PR_NUMBER: ${{ github.event.pull_request.number }}

Reference $GH_REPO and $PR_NUMBER in shell. Risk is theoretical for dotnet/maui but the env-var pattern is defense-in-depth.

if [ -z "${FILES}" ]; then
echo "No files found or API error — skipping."
exit 0
fi

HAS_ANDROID=$(echo "${FILES}" | grep -ciE '(/Android/|\.android\.cs)' || true)

@MauiBot MauiBot May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Coverage — No detection for Tizen/Linux platform files

The repo has a platform/linux label and substantial Tizen platform code (Platforms/Tizen/ directories, .tizen.cs extension files in Essentials and Controls). The workflow has no detection for these, so PRs touching Tizen/Linux code will never receive platform/linux.

Consider adding:

HAS_TIZEN=$(echo "${FILES}" | grep -ciE '(/Tizen/|\.tizen\.cs)' || true)

And:

if [ "${HAS_TIZEN}" -gt 0 ]; then
  LABELS="${LABELS} platform/linux"
fi

@MauiBot MauiBot May 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Coverage — No detection for Tizen/Linux platform files

The repo has a platform/linux label and substantial Tizen platform code (Platforms/Tizen/ dirs, .tizen.cs extension files in Essentials and Controls). PRs touching this code currently never receive platform/linux.

Add:

HAS_TIZEN=$(echo "${FILES}" | grep -ciE '(/Tizen/|\.tizen\.cs)' || true)

And apply platform/linux accordingly.

HAS_IOS=$(echo "${FILES}" | grep -ciE '(/iOS/|\.ios\.cs)' || true)
HAS_CATALYST=$(echo "${FILES}" | grep -ciE '(/MacCatalyst/|\.maccatalyst\.cs)' || true)
HAS_WINDOWS=$(echo "${FILES}" | grep -ciE '(/Windows/|\.windows\.cs)' || true)

echo "Platform file counts — Android: ${HAS_ANDROID}, iOS: ${HAS_IOS}, MacCatalyst: ${HAS_CATALYST}, Windows: ${HAS_WINDOWS}"

LABELS=""
if [ "${HAS_ANDROID}" -gt 0 ]; then
LABELS="${LABELS} platform/android"
fi
if [ "${HAS_IOS}" -gt 0 ]; then

@MauiBot MauiBot May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Correctness — .ios.cs implies both platform/ios AND platform/macos

Per MAUI conventions, .ios.cs files compile for both iOS and MacCatalyst TFMs. The current HAS_IOS variable bundles /iOS/ directory matches (iOS-only) with .ios.cs extension matches (both platforms). This means a PR that only touches .ios.cs handler files will get platform/ios but never platform/macos.

The fix requires splitting the detection:

HAS_IOS_DIR=$(echo "${FILES}" | grep -ciE '/iOS/' || true)
HAS_IOS_EXT=$(echo "${FILES}" | grep -ciE '\.ios\.cs' || true)

Then:

if [ "${HAS_IOS_DIR}" -gt 0 ] || [ "${HAS_IOS_EXT}" -gt 0 ]; then
  LABELS="${LABELS} platform/ios"
fi
if [ "${HAS_IOS_EXT}" -gt 0 ] || [ "${HAS_CATALYST}" -gt 0 ]; then
  LABELS="${LABELS} platform/macos"
fi

This way, .ios.cs extension changes add both labels, while Platforms/iOS/ directory-only changes add only platform/ios.

@MauiBot MauiBot May 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Correctness — .ios.cs implies both platform/ios AND platform/macos

Per MAUI conventions, .ios.cs files compile for both iOS and MacCatalyst TFMs. The current HAS_IOS variable bundles /iOS/ directory matches (iOS-only) with .ios.cs extension matches (both platforms). A PR that only touches .ios.cs handler files will get platform/ios but never platform/macos.

Fix by splitting detection:

HAS_IOS_DIR=$(echo "${FILES}" | grep -ciE '/iOS/' || true)
HAS_IOS_EXT=$(echo "${FILES}" | grep -ciE '\.ios\.cs' || true)

Then apply platform/ios when HAS_IOS_DIR > 0 OR HAS_IOS_EXT > 0, and platform/macos when HAS_IOS_EXT > 0 OR HAS_CATALYST > 0.

LABELS="${LABELS} platform/ios"
fi
if [ "${HAS_CATALYST}" -gt 0 ]; then
LABELS="${LABELS} platform/macos"
fi
if [ "${HAS_WINDOWS}" -gt 0 ]; then
LABELS="${LABELS} platform/windows"
fi

LABELS=$(echo "${LABELS}" | xargs)
if [ -z "${LABELS}" ]; then
echo "No platform-specific files detected — no labels applied."
exit 0
fi

# Convert space-separated to comma-separated for gh cli
LABEL_CSV=$(echo "${LABELS}" | tr ' ' ',')
echo "Applying labels: ${LABELS}"
gh pr edit "${PR_NUMBER}" --repo "${{ github.repository }}" --add-label "${LABEL_CSV}"

@MauiBot MauiBot May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Correctness — Labels are only added, never removed on synchronize

The workflow uses --add-label exclusively. When a PR is updated (e.g., git push --force rebasing away all iOS changes), stale labels like platform/ios persist permanently.

For synchronize events, the workflow should also remove platform labels that are no longer applicable. One approach:

# Remove platform labels not in current set
for label in platform/android platform/ios platform/macos platform/windows platform/linux; do
  if ! echo "${LABELS}" | grep -q "${label}"; then
    gh pr edit "${PR_NUMBER}" --repo "${GH_REPO}" --remove-label "${label}" 2>/dev/null || true
  fi
done

@MauiBot MauiBot May 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Correctness — Labels are only added, never removed on synchronize

Workflow uses --add-label exclusively. When a PR is rebased to drop platform-specific changes, stale labels persist. For synchronize events, also remove platform labels no longer applicable. Example:

for label in platform/android platform/ios platform/macos platform/windows platform/linux; do
  if ! echo "${LABELS}" | grep -q "${label}"; then
    gh pr edit "${PR_NUMBER}" --repo "${GH_REPO}" --remove-label "${label}" 2>/dev/null || true
  fi
done

Loading