Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
66 changes: 66 additions & 0 deletions .github/workflows/pixel_validation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: Pixel Schema Validation

on:
push:
branches: [develop]
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not needed, validation should only run on PR commits and not when the PR is merged. The PR won’t be merged if there are lint issues.

Copy link
Collaborator Author

@ladamski ladamski Oct 10, 2025

Choose a reason for hiding this comment

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

I don't have a personal preference here, other than to note that:

pull_request:

jobs:
pixel-validation:
runs-on: ubuntu-latest
permissions:
contents: write
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Use Node.js 20
uses: actions/setup-node@v4
with:
node-version: 20.x

- uses: actions/cache@v4
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-

- name: Checkout internal-github-asana-utils repo
uses: actions/checkout@v4
with:
token: ${{ secrets.DAXMOBILE_ANDROID_AUTOMATION }}
path: internal-github-asana-utils
repository: duckduckgo/internal-github-asana-utils

- name: Install dependencies
run: npm ci --verbose
working-directory: PixelDefinitions

# Lint step for push (fail on error), for PRs run fix instead
- name: Check or Fix lint
id: lint
run: |
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
npm run lint.fix || true
else
npm run lint
Copy link
Contributor

Choose a reason for hiding this comment

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

We can do this as part of our https://github.com/duckduckgo/Android/blob/develop/.github/workflows/ci.yml that runs on each PR merge. That would also prevent this workflow from running in parallel.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My preference would be to have the pixel linter separated out so everything pixel validation related is in one workflow. This matches what we have done on other platforms and will make it easier to then consolidate into a shareable workflow in https://github.com/duckduckgo/native-github-asana-sync once we are more stable.

Copy link
Collaborator Author

@ladamski ladamski Oct 10, 2025

Choose a reason for hiding this comment

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

Since we aren't actually committing anything, I think it makes sense to run it in parallel? I've also scoped it down to only trigger on

      paths:
      - 'PixelDefinitions/**'

... so it won't even show up on most PRs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@marcosholgado , @malmstein really appreciate your feedback here!

As @nshuba mentioned above, our longer term goal is to consolidate the common components across 6+ products into a shared one. As such I'd like to see if the current state is acceptable. To summarize:

  • package.json stays in the PixelDefinitions subdirectory
  • a separate pixel-validation.yml runs only for PR with commits in the PixelDefinitions subdirectory
  • fail on error, no auto fixing / committing

fi
working-directory: PixelDefinitions

# Commit and push changes if lint.fix was run and made changes
- name: Commit and push linter fixes (PRs only)
if: github.event_name == 'pull_request'
run: |
git config --global user.name "github-actions[bot]"
git config --global user.email "github-actions[bot]@users.noreply.github.com"
git add PixelDefinitions/
if ! git diff --cached --quiet; then
git commit -m "chore: auto-fix lint errors"
git push
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's check with the Android team if they are ok with automated commits. Windows opted into this, but Apple preferred a simple diff output on error: https://app.asana.com/1/137249556945/project/414709148257752/task/1210957466937460?focus=true

Copy link
Contributor

Choose a reason for hiding this comment

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

Bringing my comment:

  • lint should always run in PRs and develop and should be added to our pre-commit hook (this las bit needs added).
  • lint-fix should be run manually by the owner.
  • No automated commits anywhere.


- name: Run pixel validation
run: npm run validate-defs-without-formatting -- -g ../internal-github-asana-utils/user_map.yml
working-directory: PixelDefinitions

6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,8 @@ report
!/node_modules/@duckduckgo/content-scope-scripts/build/android/
!/node_modules/@duckduckgo/content-scope-scripts/build/android/pages/
/node_modules/@duckduckgo/content-scope-scripts/build/android/pages/*
!/node_modules/@duckduckgo/content-scope-scripts/build/android/pages/duckplayer/
!/node_modules/@duckduckgo/content-scope-scripts/build/android/pages/duckplayer/

# Pixel processing results
PixelDefinitions/pixel_processing_results/*
PixelDefinitions/node_modules/
1 change: 1 addition & 0 deletions PixelDefinitions/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.md
1 change: 1 addition & 0 deletions PixelDefinitions/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
For information about documenting pixels and validating them, see https://github.com/duckduckgo/pixel-schema/blob/main/README.md
5 changes: 5 additions & 0 deletions PixelDefinitions/asana_notify.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"assignee": "ladamski",
"followers": ["nshuba"],
"tagPixelOwners": false
}
1 change: 1 addition & 0 deletions PixelDefinitions/ignore_params.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
5 changes: 5 additions & 0 deletions PixelDefinitions/native_experiments.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
// This file will define pixels sent by the native experiments framework
"defaultSuffixes": ["form_factor"],
"activeExperiments": {}
}
Loading
Loading