-
Notifications
You must be signed in to change notification settings - Fork 236
chore: address barebones feedback #5852
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.
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a790a10
chore: address linting workflow feedback
rubencarvalho e7a3b99
chore: make lint workflow work in this PR
rubencarvalho 501f4a0
chore: add CSS HMR to 2nd-gen
rubencarvalho 2dc3cba
chore: fix tachometer runs
rubencarvalho 8f2a189
chore: address documentation feedback
rubencarvalho aea4c27
chore: remove barebones for CI run
rubencarvalho b3e9109
chore: skip tachometer tests
rubencarvalho 9941f2b
chore: 2nd-gen lint
rubencarvalho 9142efb
chore: apply lint fix on 2nd gen
rubencarvalho File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,99 +1,98 @@ | ||
| name: Browser Performance Tests | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
| workflow_dispatch: # Only run manually, not on pull requests | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| jobs: | ||
| test-changed-packages: | ||
| strategy: | ||
| matrix: | ||
| browser: [firefox, chrome] | ||
| name: Compare performance to latest release on ${{ matrix.browser }} | ||
|
|
||
| # The job will only run if the pull request is from the same repository. | ||
| # Benchmarks can't run on PRs from forked repos due to comment posting restrictions without a GitHub token. | ||
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Checkout main | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main | ||
|
|
||
| - name: Checkout PR branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 # Get full history | ||
|
|
||
| - name: Fetch main branch | ||
| run: | | ||
| git fetch origin main:main | ||
|
|
||
| - name: Setup Job and Install Dependencies | ||
| uses: ./.github/actions/setup-job | ||
|
|
||
| - name: Check ChromeDriver Version | ||
| if: matrix.browser == 'chrome' | ||
| run: | | ||
| echo "Checking ChromeDriver version..." | ||
| npx chromedriver --version | ||
| echo "Checking Chrome version..." | ||
| google-chrome --version | ||
| echo "Checking tachometer chromedriver version..." | ||
| cd 1st-gen && yarn tachometer --version | ||
|
|
||
| - name: Tachometer the changed packages | ||
| run: cd 1st-gen && yarn test:changed --browser=${{ matrix.browser }} | ||
|
|
||
| - name: Create a dummy file to ensure at least one results file exists | ||
| run: touch 1st-gen/tachometer.${{ matrix.browser }}-ran.txt | ||
|
|
||
| - name: Archive ${{ matrix.browser }} tachometer results | ||
| id: upload-artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: tachometer-results-${{ matrix.browser }} | ||
| path: | | ||
| 1st-gen/tach-results.${{ matrix.browser }}.*.json | ||
| 1st-gen/tachometer.${{ matrix.browser }}-ran.txt | ||
|
|
||
| comment-performance: | ||
| name: Comment tachometer performance results | ||
| needs: [test-changed-packages] | ||
|
|
||
| # The job will only run if the pull request is from the same repository. | ||
| # Benchmarks can't run on PRs from forked repos due to comment posting restrictions without a GitHub token. | ||
| if: ${{ github.event.pull_request == null || github.event.pull_request.head.repo.full_name == github.repository }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout main | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main | ||
|
|
||
| - name: Checkout PR branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 2 | ||
|
|
||
| - name: Setup Job and Install Dependencies | ||
| uses: ./.github/actions/setup-job | ||
|
|
||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| pattern: tachometer-results-* | ||
| merge-multiple: true | ||
|
|
||
| - name: Post Tachometer Performance Comment | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const { buildTachometerComment } = await import('${{ github.workspace }}/.github/scripts/build-tachometer-comment.js'); | ||
| const { commentOrUpdate } = await import('${{ github.workspace }}/.github/scripts/comment-or-update.js'); | ||
| const body = buildTachometerComment(); | ||
| commentOrUpdate(github, context, '## Tachometer results', body); | ||
| test-changed-packages: | ||
| strategy: | ||
| matrix: | ||
| browser: [firefox, chrome] | ||
| name: Compare performance to latest release on ${{ matrix.browser }} | ||
|
|
||
| # The job will only run if the pull request is from the same repository. | ||
| # Benchmarks can't run on PRs from forked repos due to comment posting restrictions without a GitHub token. | ||
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Checkout main | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main | ||
|
|
||
| - name: Checkout PR branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 # Get full history | ||
|
|
||
| - name: Fetch main branch | ||
| run: | | ||
| git fetch origin main:main | ||
|
|
||
| - name: Setup Job and Install Dependencies | ||
| uses: ./.github/actions/setup-job | ||
|
|
||
| - name: Check ChromeDriver Version | ||
| if: matrix.browser == 'chrome' | ||
| run: | | ||
| echo "Checking ChromeDriver version..." | ||
| npx chromedriver --version | ||
| echo "Checking Chrome version..." | ||
| google-chrome --version | ||
| echo "Checking tachometer chromedriver version..." | ||
| cd 1st-gen && yarn tachometer --version | ||
|
|
||
| - name: Tachometer the changed packages | ||
| run: cd 1st-gen && yarn test:changed --browser=${{ matrix.browser }} | ||
|
|
||
| - name: Create a dummy file to ensure at least one results file exists | ||
| run: touch 1st-gen/tachometer.${{ matrix.browser }}-ran.txt | ||
|
|
||
| - name: Archive ${{ matrix.browser }} tachometer results | ||
| id: upload-artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: tachometer-results-${{ matrix.browser }} | ||
| path: | | ||
| 1st-gen/tach-results.${{ matrix.browser }}.*.json | ||
| 1st-gen/tachometer.${{ matrix.browser }}-ran.txt | ||
|
|
||
| comment-performance: | ||
| name: Comment tachometer performance results | ||
| needs: [test-changed-packages] | ||
|
|
||
| # The job will only run if the pull request is from the same repository. | ||
| # Benchmarks can't run on PRs from forked repos due to comment posting restrictions without a GitHub token. | ||
| if: ${{ github.event.pull_request == null || github.event.pull_request.head.repo.full_name == github.repository }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout main | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main | ||
|
|
||
| - name: Checkout PR branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 2 | ||
|
|
||
| - name: Setup Job and Install Dependencies | ||
| uses: ./.github/actions/setup-job | ||
|
|
||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| pattern: tachometer-results-* | ||
| merge-multiple: true | ||
|
|
||
| - name: Post Tachometer Performance Comment | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const { buildTachometerComment } = await import('${{ github.workspace }}/.github/scripts/build-tachometer-comment.js'); | ||
| const { commentOrUpdate } = await import('${{ github.workspace }}/.github/scripts/comment-or-update.js'); | ||
| const body = buildTachometerComment(); | ||
| commentOrUpdate(github, context, '## Tachometer results', body); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this is passing in CI, but locally, I'm still getting a lint error:
If I change this to:
I can get it to run and it finds lint errors:
Should we address these?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I addressed this for now and pushed a commit with the
--fixed files.The more comprehensive linting fix will happen in: #5847
LMK if you still find something quirky!