This repository was archived by the owner on Oct 15, 2024. It is now read-only.
Update dependencies and do some slight refactors #1131
Merged
Conversation
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
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
…ments The newest versions of AndroidX Activity and Fragments correctly enforce the requirement for all contracts to be registered at class init or before the lifecycle has reached `Lifecycle.State.STARTED`. To comply with these requirements, move all instances of `registerForActivityResult` being called at arbitrary points in the code to be done at class init. Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
fmeum
previously approved these changes
Oct 2, 2020
Member
fmeum
left a comment
fmeum
left a comment
There was a problem hiding this comment.
Looks good to me with one suggested change. I don't like the additional class members, but also don't see a way around them.
.github/workflows/pull_request.yml
Outdated
| const serviceChanged = result.data.filter(f => f.filename.startsWith("app/") || f.filename.startsWith("buildSrc/") || f.filename.endsWith("gradle") || f.filename.startsWith(".github/workflows/pull_request.yml") || f.filename.startsWith("gradle") || f.filename.endsWith("properties")).length > 0 | ||
| console.log(serviceChanged) | ||
| return serviceChanged | ||
| const shouldRun = result.data.filter(f => !f.filename.endsWith("md") || !f.filename.endsWith("txt") || !f.filename.startsWith("contrib") || !f.filename.endsWith("yml")).length > 0 |
Member
There was a problem hiding this comment.
Suggested change
| const shouldRun = result.data.filter(f => !f.filename.endsWith("md") || !f.filename.endsWith("txt") || !f.filename.startsWith("contrib") || !f.filename.endsWith("yml")).length > 0 | |
| const shouldRun = result.data.filter(f => !f.filename.endsWith(".md") || !f.filename.endsWith(".txt") || !f.filename.startsWith("contrib/") || !f.filename.endsWith(".yml")).length > 0 |
This is going to be less work to maintain going forward since we won't need to change it every time we include a new subproject. Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
fmeum
approved these changes
Oct 2, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
📢 Type of change
📜 Description
Updates dependencies, slightly refactors instances of
registerForActivityResultand fixes the pull request workflow to correctly run in all needed scenarios.💡 Motivation and Context
Pull Requests currently do not run for changes to
autofill-parser, this fixes that and makes the workflow require less changes going forward.The API now strictly demands all contracts be eagerly registered at class init, we now comply with this requirement.
Updates to Biometric for memory leak fixes, to Activity and Fragment for new contract registration requirements and a fix for request code pool starvation, and to Core for no good reason :P
💚 How did you test it?
Verified biometrics works as before and all previously lazy registrations of activity result contracts don't trigger crashes now.
📝 Checklist
🔮 Next steps
📸 Screenshots / GIFs