-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
CI test for 16kb and non-16kb page alignment #18248
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
base: main
Are you sure you want to change the base?
Conversation
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 don't understand enough to tell anything else than that we should add a note about the original author of the new file, sorry
@mikehardy - name: Verify Page Size
if: matrix.target == 'google_apis_ps16k'
run: |
PAGE_SIZE=$(adb shell getconf PAGE_SIZE)
echo "Page size: $PAGE_SIZE"
if [ "$PAGE_SIZE" != "16384" ]; then
echo "Error: Expected page size of 16384, got $PAGE_SIZE"
exit 1
fi But when I noticed it wasn't running in the tests, I realised we can't run Is there some other way to test it? |
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.
Executes a Google script. Looks fine to me
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.
Sorry this sat so long when you were trying to close an issue I logged myself!
I think all of the ideas and the main work here are great. My comments need to be addressed but I think (hope ?) they're all easy modifications on your core work here, just to future proof it
dc08ce9
to
c44c1cd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
efccd3e
to
d46e53c
Compare
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.
closer still - but did realize that we're not actually executing tests against the google_apis_ps16k emulator target
iteration: 0 | ||
- api-level: 35 | ||
arch: x86_64 | ||
target: google_apis |
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.
Fantastic - approved, pending upstream PR merge+release and this change here
target: google_apis | |
target: google_apis_ps16k |
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.
Yep I have amended the commit already, waiting for the release to do a force push!
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.
had a review comment on that PR I just had time to address now, hopefully will release soon - maintainer there is collaborating fine and that change is needed of course
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.
Hasn't been released yet, but temporarily depending on this commit hash for the android-emulator-runner package should work, the PR is merged now ReactiveCircus/android-emulator-runner@66283c0
Purpose / Description
Updates GitHub workflow matrix to extend to 16kb page alignment.
Fixes
Approach
How Has This Been Tested?
Passes CI on fork
Learning (optional, can help others)
Refered to 16KB page alignment docs
Checklist
Please, go through these checks before submitting the PR.