Closed
Bug 1119167
Opened 10 years ago
Closed 10 years ago
Advertise IntelliJ support in build message
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla38
People
(Reporter: mhaigh, Assigned: Skandan, Mentored)
References
Details
(Whiteboard: [lang=python][good first bug])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Skandan
:
review+
|
Details | Diff | Splinter Review |
In the same way that we currently have a message advertising Eclipse support whilst running ./mach build, we should advertise IntelliJ support
Updated•10 years ago
|
OS: Mac OS X → Android
Hardware: x86 → All
Comment 2•10 years ago
|
||
Yes! Great idea. I think what we should do is replace the Android Eclipse messaging with Android IntelliJ messaging.
To address this ticket, rename the relevant message at [1] to something more generic (say, ANDROID_IDE_ADVERTISEMENT) and update the message to say something like:
'''
=============
ADVERTISEMENT
You are building Firefox for Android. After your build completes, you should run `mach gradle-install` to prepare Gradle and IntelliJ/Android Studio integration. Then import the Gradle project at $OBJDIR/mobile/android/gradle into the IDE of your choice.
PLEASE BE AWARE THAT GRADLE AND INTELLIJ/ANDROID STUDIO SUPPORT IS EXPERIMENTAL.
You should verify any changes using |mach build|.
=============
'''
[1] https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/config_status.py#29
Mentor: nalexander
Flags: needinfo?(nalexander)
Updated•10 years ago
|
Whiteboard: [lang=python][good first bug]
Assignee | ||
Comment 3•10 years ago
|
||
Does the patch do what's required or does it require some other changes as well? Please tell me if there's any changes to be made!
Attachment #8546436 -
Flags: feedback?(nalexander)
Comment 4•10 years ago
|
||
Comment on attachment 8546436 [details] [diff] [review]
Changes advertising IntelliJ
nit: Line wrap the long line to 70 or 78 characters.
nit: format the commit message like
Bug 1119167 - Advertise IntelliJ support in build-backend message. r=nalexander
And you'll need to update the reference to ANDROID_ECLIPSE_ADVERTISEMENT below, where it is used. Looks like you haven't tested with your changes: to do so, run
./mach build-backend
with a Fennec mozconfig in place (see https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec for how to get such a configuration set up).
First cut is looking hopeful.
Attachment #8546436 -
Flags: feedback?(nalexander) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
:-) Sorry that was a stupid mistake to make. I have corrected the nits. My android device had trouble mounting. So couldn't check it last time. I managed to get it mounted, now will build fennec and check. Thank You!
Assignee | ||
Comment 6•10 years ago
|
||
Hey Nick, I've made the changes the new patch, but I'm not able to check it out by building. My fennec build is running into some problems, so I havent been able to build as yet. But I'm working on configuring it out properly ASAP. Sorry! Anything I should do?
Attachment #8546436 -
Attachment is obsolete: true
Attachment #8547004 -
Flags: feedback?(nalexander)
Assignee | ||
Comment 7•10 years ago
|
||
I set up my fennec and ran ./mach build-backend and the advertisement did appear.
Comment 8•10 years ago
|
||
Comment on attachment 8547004 [details] [diff] [review]
Bug-1119167.diff(Changes added)
Review of attachment 8547004 [details] [diff] [review]:
-----------------------------------------------------------------
Looking good! Just one small nit. When you upload the new patch, carry-over the r+ and set the "checkin-needed" flag in Bugzilla. That'll get a sheriff to land your patch for you.
Sheriff: this patch doesn't need a try run.
::: python/mozbuild/mozbuild/config_status.py
@@ +30,4 @@
> =============
> ADVERTISEMENT
>
> +You are building Firefox for Android. After your build completes, you
nit: delete trailing whitespace throughout.
Attachment #8547004 -
Flags: review+
Attachment #8547004 -
Flags: feedback?(nalexander)
Attachment #8547004 -
Flags: feedback+
Updated•10 years ago
|
Assignee: nobody → prathikcoding167
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•10 years ago
|
||
White space nits removed. Thank You for the opportunity!!
Attachment #8547004 -
Attachment is obsolete: true
Attachment #8547287 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 10•10 years ago
|
||
Sorry, the previous uploaded was an obsolete one. This is the correct one.
Attachment #8547287 -
Attachment is obsolete: true
Attachment #8547288 -
Flags: review+
Comment 11•10 years ago
|
||
can we get a try run to make sure nothing brakes etc ? Thanks!
Flags: needinfo?(prathikcoding167)
Keywords: checkin-needed
Assignee | ||
Comment 12•10 years ago
|
||
Yeah I tried another run and it's working fine. Did you mean I should try running it someplace else online? Sorry cause I'm new to this. Ping me if there's something I must do. :)
Flags: needinfo?(prathikcoding167)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 13•10 years ago
|
||
He was referring to a Tryserver run:
https://wiki.mozilla.org/ReleaseEngineering/TryServer
Basically, it gives us a chance to run your patch through an environment configured exactly like our production automation before landing to ensure that nothing will break when it's pushed. That said, this looks like trivial enough that I'm going to go ahead and push it without one this time.
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d171761d10c6
Thanks for the patch!
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Updated•5 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 38 → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•