552190 - Reorder package files

Status: RESOLVED INCOMPLETE

(SeaMonkey :: Build Config, defect)

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) removed-files.in, and remove *sqlite3* for all platforms

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) some of package-manifest.in

Comment 3

[Justin Wood (:Callek)]

Comment on attachment 432332 [details] [diff] [review]
(Av1) removed-files.in, and remove *sqlite3* for all platforms

>+@DLL_PREFIX@sqlite3@DLL_SUFFIX@

Probably actually belongs where you put it, but was inside an XP_WIN ifdef before you moved it here. Can you please verify for me that this ifdef breakout is intended.

Feedback to KaiRo just for a rubber stamp on the reordering, the patch is good, in order and not missing anything (except what I said above).

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 432333 [details] [diff] [review]
(Bv1) some of package-manifest.in

> @BINPATH@/run-mozilla.sh
> #endif
>+
> #ifndef MOZ_STATIC_BUILD
> #endif

Nit: no blank line here.

> ; MailNews chrome
>-@BINPATH@/chrome/messenger.jar
>-@BINPATH@/chrome/messenger.manifest
>-@BINPATH@/chrome/newsblog.jar
>-@BINPATH@/chrome/newsblog.manifest

Please keep these here, to me they belong in this spot, with this comment.

Other than that, also looks good. To me personally with all the scattered ifdefs this does make it harder to read; but I don't have a strong opinion. feedback to KaiRo for rubber stamp again though.

Comment 5

[Robert Kaiser]

So, I guess we're reordering those files monthly now. I had reordered them when I merged the packaging files. In any case, I'll take a look once I find time and motivation, which could be anywhere between hours and months from now.

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 432332 [details] [diff] [review]
(Av1) removed-files.in, and remove *sqlite3* for all platforms


(In reply to comment #3)
> >+@DLL_PREFIX@sqlite3@DLL_SUFFIX@

I'm sorry I did not comment on this: it was put in the wrong place in bug 513747.

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)

> Nit: no blank line here.

Agreed.

> Please keep these here, to me they belong in this spot, with this comment.

The idea is to sort this block where 'm' and 'n' come after 'i', shouldn't they?

(In reply to comment #5)
> So, I guess we're reordering those files monthly now.

Just doing it incrementally...

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1a) some of package-manifest.in

Bv1, with first comment 4 suggestion (only),
and unbitrotted after bug 549293.

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) removed-files.in [Checkin: See comment 14]

Av1, unbitrotted after bug 549293.

Comment 10

[Robert Kaiser]

Comment on attachment 434523 [details] [diff] [review]
(Bv1a) some of package-manifest.in

please don't put ANYTHING between application.ini and platform.ini - the binary/script and those two INIs are one block for themselves for a reason.

Also, I don't think any reordering bug makes ANY sense at all unless you provide an explanation, probably even a wiki page, describing in what way things *should* be ordered. Those files are _very_ hard to read anyhow with the countless ifdefs, I'd rather try to reduce the number of those, for example.

Comment 11

[Robert Kaiser]

Comment on attachment 434525 [details] [diff] [review]
(Av1a) removed-files.in
[Checkin: See comment 14]

>+#ifdef XP_MACOSX
>+XUL
>+#endif

Most of what you are doing looks reasonable here, I at least can derive some systematics, but this one is at the wrong place by whatever rules I come up with. Care to explain or even document what rules we ought to be applying?

Still, everything but this one makes sense for me here.

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> Care to explain or even document what rules we ought to be applying?

I'm trying to be closer to alphabetical with ignoring @ prefix and suffix...
Please suggest where you'd like 'XUL' to be.

Comment 13

[Robert Kaiser]

(In reply to comment #12)
> I'm trying to be closer to alphabetical with ignoring @ prefix and suffix...

In removed-files.in, I see how you mostly are.

> Please suggest where you'd like 'XUL' to be.

I guess the best position would be right after "@DLL_PREFIX@xul@DLL_SUFFIX@" (right before it breaks that block of items so it'd be harder to read).

Comment 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 434525 [details] [diff] [review]
(Av1a) removed-files.in
[Checkin: See comment 14]


http://hg.mozilla.org/comm-central/rev/a68d53a5c31c
Av1a, with comment 13 suggestion(s).


(In reply to comment #13)

(Ftr, I had put it before the block, because 'X' is sorted before 'g'.)

Comment 15

[Robert Kaiser]

(In reply to comment #14)
> (Ftr, I had put it before the block, because 'X' is sorted before 'g'.)

Oh, we generally do case-insensitive sorting.

Comment 16

[Frank-Rainer Grahl (:frg)]

The current structure matches Firefox and TB more or less so lest close it.