Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Switch to GeckoView 69 Beta. #4297

Merged
merged 1 commit into from Jul 30, 2019
Merged

Switch to GeckoView 69 Beta. #4297

merged 1 commit into from Jul 30, 2019

Conversation

pocmo
Copy link
Contributor

@pocmo pocmo commented Jul 25, 2019

This is currently being discussed. Please only merge if there's a decision to ship with GV 69 Beta.

Engineering-wise with GeckoView 69/Beta we no longer need to maintain a copy of the GeckoView version in Fenix and so I deleted Gecko.kt. Just including browser-engine-gecko-beta is enough and the matching GeckoView version will come automatically as transitive dependency.

@mitchhentges
Copy link
Contributor

Heads up that this will break raptor tasks, since this affects our gradle function for returning the current geckoview release.

See reference-browser's printGeckoviewVersion function. If you update fenix's build.gradle to have the same function body, raptor should still work. This can be tested locally with ./gradlew printGeckoviewVersions.
Let me know if you'd like assistance, or if you'd like me to update it and provide you a patch/update your branch

@pocmo
Copy link
Contributor Author

pocmo commented Jul 30, 2019

@mitchhentges Thanks! I'll fix that and flag you for review.

@mozilla-mobile/fenix This PR is ready to get in - all stakeholders seem to agree that we want to go with GV 69 as soon as possible.

@pocmo
Copy link
Contributor Author

pocmo commented Jul 30, 2019

@mitchhentges I updated printGeckoviewVersion (also without s now). I had to add a check for the selected property since it didn't seem to exist for unresolved dependencies. I couldn't find any code in Fenix yet that actually calles printGeckoviewVersion/s - is that right?

@pocmo pocmo requested a review from mitchhentges July 30, 2019 09:07
@mitchhentges
Copy link
Contributor

mitchhentges commented Jul 30, 2019

@pocmo thanks!

I had to add a check for the selected property since it didn't seem to exist for unresolved dependencies

IIRC by fetching resolutionResult, you trigger the resolution of the entire resolution graph. Is it possible for there to be unresolved dependencies at that point?

I couldn't find any code in Fenix yet that actually calles printGeckoviewVersion/s - is that right?

... wait a second 🤦‍♂️ we always use the latest raptor these days! Since we aren't matching the raptor version with the geckoview version, you can safely remove printGeckoviewVersion. Sorry!

@pocmo
Copy link
Contributor Author

pocmo commented Jul 30, 2019

IIRC by fetching resolutionResult, you trigger the resolution of the entire resolution graph. Is it possible for there to be unresolved dependencies at that point?

I'm not sure. 🤷‍♂ It ended up calling selected on some Unresolved.... class that didn't have this property. 🤔

... wait a second 🤦‍♂ we always use the latest raptor these days! You can safely remove printGeckoviewVersion, my bad

Ah, great. Will do that!

@pocmo pocmo merged commit d1d91e9 into mozilla-mobile:master Jul 30, 2019
@pocmo pocmo deleted the gv-69-beta branch July 30, 2019 16:53
JohanLorenzo added a commit to mozilla-releng/staging-fenix that referenced this pull request Jul 22, 2022
JohanLorenzo added a commit to JohanLorenzo/fenix that referenced this pull request Jul 25, 2022
JohanLorenzo added a commit that referenced this pull request Jul 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants