Closed Bug 924738 Opened 6 years ago Closed 6 years ago

Move third party sources outside of base

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 27

People

(Reporter: bnicholson, Assigned: bnicholson)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

Having all third party source directly under base (which is generally o.m.g.) is not IDE-friendly, so we should move them outside. This overlaps with bug 890586, but is less ambitious.

Here are the shell commands for all file moves, as requested by rnewman:
git mv httpclientandroidlib/ ../thirdparty/
git mv json-simple/ ../thirdparty/
git mv apache/ ../thirdparty/
git mv braille/ ../thirdparty/
git mv websockets/ ../thirdparty/

Thanks in advance for nalexander or rnewman for syncing these changes with the services branch :)
Blocks: 890586
Status: NEW → ASSIGNED
Attachment #814711 - Flags: review?(rnewman)
See Also: → 890586
Oops, dependency works too.
See Also: 890586
I was going to keep this bug as simple as possible, but we want this change too if we really want to make IDE integration painless.

cd thirdparty/
git mv braille/com/ .
rm -r braille/
mkdir -p org/mozilla
git mv apache/ org/mozilla/
mkdir -p ch/boye
git mv httpclientandroidlib/ ch/boye/
mkdir -p com/codebutler
git mv websockets/ com/codebutler/android_websockets
mkdir org/json
git mv json-simple/ org/json/simple
Attachment #814713 - Flags: review?(rnewman)
In principle, I support this.  And, to my mind, in for a penny, in for a pound: we should follow Java's directory structure everywhere.

I have a few nits with the build system integration, and I'd kind of prefer to land Bug 923306 before doing this rename the world operation, but f+ from me.
Let's wait for the build bug to land before we do this, 'cos I think this is easier to reconstitute. Work for you, Brian?
Depends on: 923306
WFM
Rebased
Attachment #814711 - Attachment is obsolete: true
Attachment #814711 - Flags: review?(rnewman)
Attachment #817379 - Flags: review?(rnewman)
Rebased
Attachment #814713 - Attachment is obsolete: true
Attachment #814713 - Flags: review?(rnewman)
Attachment #817380 - Flags: review?(rnewman)
Comment on attachment 817380 [details] [diff] [review]
0002-Bug-924738-Use-package-hierarchy-for-third-party-lib.patch, v2

These look fine to me, but needs build peer review.
Attachment #817380 - Flags: review?(rnewman)
Attachment #817380 - Flags: review?(gps)
Attachment #817380 - Flags: review+
Comment on attachment 817379 [details] [diff] [review]
0001-Bug-924738-Move-third-party-sources-outside-of-base.patch, v2

Fold these together?
Attachment #817379 - Flags: review?(rnewman)
Attached patch Part 3Splinter Review
This updates java-third-party-sources.mn, which isn't currently used in the build.
Attachment #817555 - Flags: review+
Upstream copy script change:

https://github.com/mozilla-services/android-sync/pull/365

Verified that with the three hg parts applied, this is a no-op when run.
Comment on attachment 817380 [details] [diff] [review]
0002-Bug-924738-Use-package-hierarchy-for-third-party-lib.patch, v2

Review of attachment 817380 [details] [diff] [review]:
-----------------------------------------------------------------

IMO this counts as trivial and didn't require build peer review.
Attachment #817380 - Flags: review?(gps) → review+
Nick wants to wait for bug 925185 before we land this one.
Depends on: 925185
https://hg.mozilla.org/mozilla-central/rev/070ebdc4a8e4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.