Closed Bug 1128362 Opened 7 years ago Closed 6 years ago

Add gecko libraries Gradle project

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(9 files, 1 obsolete file)

39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
39 bytes, text/x-review-board-request
Details
This ticket tracks adding a gecko libraries Gradle project that contains the .so libraries in assets/ and lib/.

The idea is to do two things: first, be more deliberate about copying and packaging compiled libraries from dist/.  In this respect, this new libraries project mirrors the omnijar project.  Second, provide a named dependency that we can download rather than produce locally in the future.
/r/3241 - Bug 1128362 - Add gecko_libs Gradle project. r=me

Pull down this commit:

hg pull review -r 623ba1bfde62d52d31b858a8a81042e94c91ef93
https://reviewboard.mozilla.org/r/3241/#review2655

Always look on the bright side of life.

I analyzed your Python changes and found 2 errors.

The following files were examined:

  mobile/android/mach_commands.py

::: mobile/android/mach_commands.py
(Diff revision 1)
> +        srcdir('gecko_libs/build.gradle', 'mobile/android/gradle/gecko_libs/build.gradle')

E501: line too long (90 > 79 characters)
Limit all lines to a maximum of 79 characters.

There are still many devices around that are limited to 80 character
lines; plus, limiting windows to 80 characters makes it possible to have
several windows side-by-side.  The default wrapping on such devices looks
ugly.  Therefore, please limit all lines to a maximum of 79 characters.
For flowing long blocks of text (docstrings or comments), limiting the
length to 72 characters is recommended.

Reports error E501.

::: mobile/android/mach_commands.py
(Diff revision 1)
> +        srcdir('gecko_libs/src/main/AndroidManifest.xml', 'mobile/android/gradle/gecko_libs/AndroidManifest.xml')

E501: line too long (113 > 79 characters)
Limit all lines to a maximum of 79 characters.

There are still many devices around that are limited to 80 character
lines; plus, limiting windows to 80 characters makes it possible to have
several windows side-by-side.  The default wrapping on such devices looks
ugly.  Therefore, please limit all lines to a maximum of 79 characters.
For flowing long blocks of text (docstrings or comments), limiting the
length to 72 characters is recommended.

Reports error E501.
Comment on attachment 8557684 [details]
MozReview Request: bz://1128362/nalexander

/r/3241 - Bug 1128362 - Part 1: Share generateCodeAndResources. r=me
/r/6443 - Bug 1128362 - Part 2: Copy omni.ja from dist/fennec manually. r=me
/r/6445 - Bug 1128362 - Part 3: Make running Robocop from IntelliJ work. r=me
/r/6447 - Bug 1128362 - Part 4: Move some build steps earlier. r=gps
/r/6449 - Bug 1128362 - Part 5: Use pre-built geckolibs dependency in debug configuration. r=me
/r/6451 - Bug 1128362 - Part 6: Make Gradle's release configuration work like the old debug configuration. r=me

Pull down these commits:

hg pull -r 97c890d9a7bbd043d9acfac8bbce92464578d478 https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8557684 [details]
MozReview Request: bz://1128362/nalexander

/r/3241 - Bug 1128362 - Part 1: Share generateCodeAndResources. r=me
/r/6443 - Bug 1128362 - Part 2: Copy omni.ja from dist/fennec manually. r=me
/r/6445 - Bug 1128362 - Part 3: Make running Robocop from IntelliJ work. r=me
/r/6447 - Bug 1128362 - Part 4: Move some build steps earlier. r=gps
/r/6449 - Bug 1128362 - Part 5: Use pre-built geckolibs dependency in debug configuration. r=me
/r/6451 - Bug 1128362 - Part 6: Make Gradle's release configuration work like the old debug configuration. r=me
/r/6633 - Fix
/r/6635 - Bug 1112673 - Part 1: Refactor shared footer functionality into RemoteTabsBaseFragment. r=margaret
/r/6637 - Bug 1112673 - Part 2: Add hidden devices hint to Remote Tabs panel footer. r=margaret

Pull down these commits:

hg pull -r 36bbe078391d43fbfd3f00e00630764e8228ca18 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8557684 - Flags: review?(margaret)
psd: thanks for the commits!  I had some issues with yours (not huge) and wanted to rework the visuals a bit, so I took your work and ran with it.  Care to test this and provide feedback?

margaret: could you review the last two commits?  I don't know how to edit the RB request to be more sensible.
Flags: needinfo?(prabhjyotsingh95)
Flags: needinfo?(margaret.leibovic)
antlam: here's the visual:

https://people.mozilla.org/~nalexander/screenshots/Hidden.Devices.Tip.png
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Flags: needinfo?(alam)
Hm, I kind of question whether this bit of information is suitable to be displayed in our snippets. It seems too prominent. But maybe I need more context. 

Some questions:
 - How many devices do most of our users have synced?
 - Why is this ability to "hide" really that important to call out? After all, long-press is common amongst our Panels
 - We already have a snippet overload situation, it might be best to be more conservative when adding another.

From a UX point of view, the snippet isn't really used (anywhere else) to show "tips" on a per Panel basis. Rather, we display "tips" in the empty state of Panels so that's seems more fitting. OR maybe we need to design a non-empty state way to display tips in our Panels.
Flags: needinfo?(alam) → needinfo?(nalexander)
Sorry, this all is in the wrong bug because I follow links from HG and RB and I didn't make the review request correctly.  I'm going to manually move comments and request over.
Flags: needinfo?(nalexander)
Flags: needinfo?(margaret.leibovic)
Depends on: 1160357
Flags: needinfo?(prabhjyotsingh95)
Attachment #8557684 - Attachment is obsolete: true
Attachment #8557684 - Flags: review?(margaret)
Attachment #8619270 - Flags: review?(margaret)
Attachment #8619271 - Flags: review?(margaret)
Attachment #8619272 - Flags: review?(margaret)
Attachment #8619273 - Flags: review?(margaret)
Attachment #8619274 - Flags: review?(margaret)
Attachment #8619275 - Flags: review?(margaret)
Attachment #8619276 - Flags: review?(margaret)
Attachment #8619277 - Flags: review?(margaret)
Attachment #8619278 - Flags: review?(margaret)
We're not going to take this approach; we've gone in a different direction with Bug 1233882.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Attachment #8619270 - Flags: review?(margaret)
Attachment #8619271 - Flags: review?(margaret)
Attachment #8619272 - Flags: review?(margaret)
Attachment #8619273 - Flags: review?(margaret)
Attachment #8619274 - Flags: review?(margaret)
Attachment #8619275 - Flags: review?(margaret)
Attachment #8619276 - Flags: review?(margaret)
Attachment #8619277 - Flags: review?(margaret)
Attachment #8619278 - Flags: review?(margaret)
Component: Build Config → Build Config & IDE Support
Product: Core → Firefox for Android
Product: Firefox for Android → Firefox Build System
You need to log in before you can comment on or make changes to this bug.