Closed
Bug 936468
Opened 6 years ago
Closed 6 years ago
Move OS: Windows 8 Metro to Windows 8.1
Categories
(bugzilla.mozilla.org :: Administration, task, P2)
P2
Tracking
()
RESOLVED
FIXED
People
(Reporter: jimm, Assigned: dylan)
Details
Attachments
(1 file, 3 obsolete files)
2.66 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
This was originally created to isolate metro related bugs from product firefox. Later added a new product, so this isn't needed anymore. We would like to delete this os moving any bug under it to os: Windows 8.1 (hopefully without generating any bugspam?). This change was discussed on the metro public mailing list.
Comment 1•6 years ago
|
||
i've updated the field value so it's no longer an option. we'll have to write a script to update the 1962 bugs currently using that o/s.
Updated•6 years ago
|
Assignee: nobody → dylan
Assignee | ||
Comment 2•6 years ago
|
||
This is decidedly a one-off script. Current behavior is to reset the OS field to 'Other', I'm not sure what else would be defined behavior.
Attachment #8439730 -
Flags: review?(glob)
Assignee | ||
Updated•6 years ago
|
Status: NEW → ASSIGNED
Comment 3•6 years ago
|
||
Comment on attachment 8439730 [details] [diff] [review] bug-936468-v1.patch Review of attachment 8439730 [details] [diff] [review]: ----------------------------------------------------------------- ::: contrib/reorg-tools/remove-os.pl @@ +8,5 @@ > + > +use 5.10.1; > +use strict; > +use warnings; > +use lib qw(. lib); while most reorg script don't current do this, i think it's nicer to allow these scripts to work regardless of the cwd: use FindBin '$RealBin'; use lib "$ReadBin/../..", "$RealBin/../../lib"; @@ +32,5 @@ > + > +$dbh->bz_start_transaction; > + > +my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); > +my $field = Bugzilla::Field->new({name => 'op_sys', cache => 1}); nit: add spaces after { and before } i'd like to see a sanity check here that default_os exists in the db (stranger things have happened!). check the result of Bugzilla::Field::Choice->type($field)->match({ value => $DEFAULT_OS }) @@ +35,5 @@ > +my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); > +my $field = Bugzilla::Field->new({name => 'op_sys', cache => 1}); > + > +my $bugs = $dbh->selectall_arrayref(q{SELECT bug_id FROM bugs WHERE bugs.op_sys = ?}, > + { Slice => {} }, $os); rather than slicing to a single element hashref, use selectcol_arrayref @@ +42,5 @@ > + my $bug_id = $bug->{bug_id}; > + warn "Removing OS '$os' from $bug_id...\n"; > + $dbh->do(q{INSERT INTO bugs_activity(bug_id, who, bug_when, fieldid, removed, added) > + VALUES (?, ?, ?, ?, ?, ?)}, > + undef, $bug_id, 1, $timestamp, $field->id, $os, $DEFAULT_OS); don't hard code the user-id. load the nobody user explicitly with $user = Bugzilla::User->check({ name => 'nobody@mozilla.org' }) @@ +63,5 @@ > + remove-os.pl --os 'Windows 8 Metro' > + > +=head1 AUTHOR > + > +Dylan William Hardison <dylan@mozilla.com> remove the author section
Attachment #8439730 -
Flags: review?(glob) → review-
Assignee | ||
Comment 4•6 years ago
|
||
And here is something I was working on when I was sick. Note that using FindBin under taint mode requires untainting $RealBin.
Attachment #8439730 -
Attachment is obsolete: true
Attachment #8454233 -
Flags: review?(glob)
Assignee | ||
Comment 5•6 years ago
|
||
Ignore the previous version, pretty sure I made a mistake there.
Attachment #8454233 -
Attachment is obsolete: true
Attachment #8454233 -
Flags: review?(glob)
Attachment #8454235 -
Flags: review?(glob)
Comment 6•6 years ago
|
||
Comment on attachment 8454235 [details] [diff] [review] bug-936468-v2.1.patch Review of attachment 8454235 [details] [diff] [review]: ----------------------------------------------------------------- ::: contrib/reorg-tools/remove-os.pl @@ +1,1 @@ > +#!/usr/bin/perl -wT nit: don't need both -w and "use warnings"; remove -w from here. @@ +11,5 @@ > +use warnings; > + > +use FindBin '$RealBin'; > +BEGIN { ($RealBin) = $RealBin =~ /^(.+)$/ } > +use lib "$RealBin/../..", "$RealBin/../../lib"; as it's a command line tool with a single source of user input, i'm not convinced the acrobatics required to work around taint issues are worth enabling of taint checking. i'll leave it up to you if you want to leave taint mode enabled. @@ +34,5 @@ > +trick_taint($os); > + > +my $check_default_os = Bugzilla::Field::Choice->type('op_sys')->match({ value => $DEFAULT_OS }); > +die "This should not happen: Cannot remove $os because $DEFAULT_OS doesn't exist.\n" > + unless @$check_default_os == 1; it's probably a good idea to also check that $os is a valid op_sys. currently if you pass in an invalid OS no output is produced by the script, which is confusing. nit: "This should not happen: " isn't necessary for error messages... errors in general shouldn't happen :) @@ +40,5 @@ > +my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); > +my $field = Bugzilla::Field->check({ name => 'op_sys', cache => 1 }); > +my $nobody = Bugzilla::User->check({ name => 'nobody@mozilla.org', cache => 1 }); > +my $bug_ids = $dbh->selectcol_arrayref(q{SELECT bug_id FROM bugs WHERE bugs.op_sys = ?}, undef, $os); > + please add a confirmation prompt here as this script could be destructive (eg. as per movebugs.pl)
Attachment #8454235 -
Flags: review?(glob) → review-
Updated•6 years ago
|
Priority: -- → P2
![]() |
Reporter | |
Comment 7•6 years ago
|
||
Comment on attachment 8454235 [details] [diff] [review] bug-936468-v2.1.patch Review of attachment 8454235 [details] [diff] [review]: ----------------------------------------------------------------- ::: contrib/reorg-tools/remove-os.pl @@ +23,5 @@ > +use Pod::Usage; > + > +Bugzilla->usage_mode(USAGE_MODE_CMDLINE); > + > +my $DEFAULT_OS = 'Other'; In the case of the metro bugs, I'd prefer they get moved to Windows 8.1. Otherwise I'm still going to have to spam the entire list updating to that. :/ Maybe this script should accept an optional command line param specifying the os to switch bug into?
Assignee | ||
Comment 8•6 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #7) > In the case of the metro bugs, I'd prefer they get moved to Windows 8.1. > Otherwise I'm still going to have to spam the entire list updating to that. > :/ Maybe this script should accept an optional command line param specifying > the os to switch bug into? This would be a more useful script to have in the future, and is trivial to implement. Good call.
Assignee | ||
Updated•6 years ago
|
Summary: Remove OS: Windows 8 Metro → Move OS: Windows 8 Metro to Windows 8.1
Assignee | ||
Comment 9•6 years ago
|
||
While we had agreed this wouldn't need another round of review, given that the nature of the script changed slightly (movement rather than removal) I figured it warranted another r?
Attachment #8454235 -
Attachment is obsolete: true
Attachment #8457737 -
Flags: review?(glob)
Assignee | ||
Comment 10•6 years ago
|
||
Or, rather -- given that the nature of this script changed, it deserves additional sanity checking than would be warranted.
Comment 11•6 years ago
|
||
Comment on attachment 8457737 [details] [diff] [review] bug-936468-v3.patch Review of attachment 8457737 [details] [diff] [review]: ----------------------------------------------------------------- r=glob ::: contrib/reorg-tools/move_os.pl @@ +10,5 @@ > +use strict; > +use warnings; > + > +use FindBin '$RealBin'; > +BEGIN { ($RealBin) = $RealBin =~ /^(.+)$/ } you don't need to detaint $RealBin
Attachment #8457737 -
Flags: review?(glob) → review+
Assignee | ||
Comment 12•6 years ago
|
||
To gitolite3@git.mozilla.org:webtools/bmo/bugzilla.git b1f98ae..10bd960 master -> master
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•