104953 - installed-chrome.txt file is picky about ordering.

Status: RESOLVED WORKSFORME

(SeaMonkey :: Themes, defect)

Description

[Joseph Elwell]

The installed-chrome.txt file wants the skin,select,install line to be at the
bottom.

patch is coming.

Comment 1

[Joseph Elwell]

Attachment: need reviews.

Comment 2

[Joseph Elwell]

inheriting bugscape info. cc'ing simon and jj

simon, jj: can I get an r, sr ASAP?

Comment 3

[Simon Fraser [no longer active]]

+    open(CHROMEFILE, ">${inst_chrome}") || die "Error: Failed to open 
$inst_chrome\n";
+    print (CHROMEFILE $filecontents);
+    close(CHROMEFILE) || die "Error: Failed to close $inst_chrome\n";
+    
     open(CHROMEFILE, ">>${inst_chrome}") || die "Error: Failed to open 
$inst_chrome\n";
     print(CHROMEFILE "${chrome_entry}\n");

Why do you open the file, write to it, close it, open again in append mode, and 
write again? Why not just open the file once?

Comment 4

[Joseph Elwell]

Attachment: ok, new patch.

Comment 5

[Simon Fraser [no longer active]]

Comment on attachment 53677 [details] [diff] [review]
ok, new patch.

sr=sfraser

Comment 6

[Joseph Elwell]

peterv: I hear you are an appropriate person to get an r= from for this module. :)

Comment 7

[Jaime Rodriguez, Jr.]

pls check this in for 094 = PDT+

Comment 8

[Joseph Elwell]

fix checked into the branch. will apply to the trunk after i pull/rebuild there.


jj gave me an r= in an IM conversation. :)

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 53677 [details] [diff] [review]
ok, new patch.

>+    my $skinselect = "";
>+    my $set = 0;

Do you really need both? It seems $set == 1 is the same as $skinselect != "".

>+    print (CHROMEFILE $filecontents);
>     print(CHROMEFILE "${chrome_entry}\n");

Don't add a space after print.

Only nits, r=peterv regardless.

Comment 10

[Joseph Elwell]

pulling off PDT radar, as this is fixed on the branch. i'm working on getting a
trunk build going on my OS X machine to get this into the trunk as well.

Comment 11

[Henrik Lynggaard Hansen]

Any progress on getting this in the trunk, if it is still relevant ? and
possibly the 1.3b branch?

Comment 12

[Simon Paquet [:sipaq]]

As far as I can see, the file this was patches against does no longer exist.
Marking WFM.