Closed Bug 1119167 Opened 5 years ago Closed 5 years ago

Advertise IntelliJ support in build message

Categories

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

37 Branch
All
Android
defect
Not set

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)

In the same way that we currently have a message advertising Eclipse support whilst running ./mach build, we should advertise IntelliJ support
nalexander, can this be a mentor bug?
Flags: needinfo?(nalexander)
OS: Mac OS X → Android
Hardware: x86 → All
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)
Whiteboard: [lang=python][good first bug]
Attached patch Changes advertising IntelliJ (obsolete) — Splinter Review
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 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+
:-) 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!
Attached patch Bug-1119167.diff(Changes added) (obsolete) — Splinter Review
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)
I set up my fennec and ran ./mach build-backend and the advertisement did appear.
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+
Assignee: nobody → prathikcoding167
Status: NEW → ASSIGNED
White space nits removed. Thank You for the opportunity!!
Attachment #8547004 - Attachment is obsolete: true
Attachment #8547287 - Flags: review+
Keywords: checkin-needed
Sorry, the previous uploaded was an obsolete one. This is the correct one.
Attachment #8547287 - Attachment is obsolete: true
Attachment #8547288 - Flags: review+
can we get a try run to make sure nothing brakes etc ? Thanks!
Flags: needinfo?(prathikcoding167)
Keywords: checkin-needed
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)
Keywords: checkin-needed
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.
https://hg.mozilla.org/mozilla-central/rev/d171761d10c6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
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.