Initial support for pixel registry #6878
Conversation
ladamski
force-pushed
the
la/pixel-schema
branch
from
e1030b3 to
700ea42
Compare
This reverts commit 7e35fd0.
| # 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 | ||
| 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 |
There was a problem hiding this comment.
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
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "assignee": "ladamski", | |||
| "followers": ["jeannamatthews", "nshuba", "sammacbeth"], | |||
There was a problem hiding this comment.
Do Sam and Jeanna want to be notified for Android failures?
| "form_factor": { | ||
| "type": "string", | ||
| "description": "Device form-factor", | ||
| "enum": ["phone", "tablet"] | ||
| }, | ||
| "platform": { | ||
| "type": "string", | ||
| "description": "Device platform", | ||
| "enum": ["android"] | ||
| }, |
There was a problem hiding this comment.
Nit (I know other platforms do this too), but we can simplify this into something like:
| "form_factor": { | |
| "type": "string", | |
| "description": "Device form-factor", | |
| "enum": ["phone", "tablet"] | |
| }, | |
| "platform": { | |
| "type": "string", | |
| "description": "Device platform", | |
| "enum": ["android"] | |
| }, | |
| "form_factor": { | |
| "type": "string", | |
| "description": "Platform and device form-factor", | |
| "key": "android", | |
| "enum": ["phone", "tablet"] | |
| }, |
| { | ||
| "epbf_android": { | ||
| "description": "User sent broken site report", | ||
| "owners": [], |
There was a problem hiding this comment.
I think this will fail validation with the latest schema release (owners should not be empty). Maybe Kate can be the owner for Android breakage pixels? Let's ask her in Asana.
| "description": "Device platform", | ||
| "enum": ["android"] | ||
| }, | ||
| "channel": { |
There was a problem hiding this comment.
Similar to other comments: is channel utilized on Android? If not, we should remove from here, params_dictionary and TEMPALTE.
| }, | ||
| "remoteConfigEtag": { | ||
| "key": "remoteConfigEtag", | ||
| "description": "", |
There was a problem hiding this comment.
Some minimal description here would help set a better precedent :) (and again, I know we have the same issue in other repos)
| @@ -0,0 +1,5 @@ | |||
| { | |||
| // This file will define pixels sent by the native experiments framework | |||
| "defaultSuffixes": [], | |||
There was a problem hiding this comment.
We can prefill this with the .android.[phone|tablet] suffix
Task/Issue URL: https://app.asana.com/1/137249556945/project/1171671347221384/task/1210907475542624
Description
Initial support for https://app.asana.com/1/137249556945/project/1209805270658160/list/1209805324244987
Specific subtask for this PR: https://app.asana.com/1/137249556945/task/1210907475542626
Steps to test this PR
https://app.asana.com/1/137249556945/task/1209901985192016?focus=true using ".../Android/PixelDefinitions/" as the target directory where specified
Feature 1
UI changes