1312406 - Remove tab groups migration code

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Banner (:standard8)]

I discovered via eslint that one of the tests (test_TabGroupsMigrator.js) isn't fully testing everything it is meant to.

However, in talking with Gijs, this code can be removed now as it was temporary code to provide users a way forward if they had previously been using tab groups.

Comment 1

[Luís Miguel [:quicksaver] (not reading bugmail)]

Is this for the mentioned test only? If not, it could also take care of bug 1255361.

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1312406 - part 1: remove tab groups migration code,

Review commit: https://reviewboard.mozilla.org/r/89824/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89824/

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1312406 and bug 1255361 - part 2: remove tab groups backup,

Review commit: https://reviewboard.mozilla.org/r/89826/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89826/

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1312406 - part 3: bonus - remove dead icon,

Review commit: https://reviewboard.mozilla.org/r/89828/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89828/

Comment 5

[:Gijs (he/him)]

(In reply to Luís Miguel [:quicksaver] from comment #1)
> Is this for the mentioned test only? If not, it could also take care of bug
> 1255361.

No, I'm removing everything. It's been a full ESR cycle. People should have hit this code by now if they were going to. :-)

Comment 6

[:Gijs (he/him)]

Small update because Yoric tells me on IRC individual OS.File operations already block shutdown, so no need to manually add a shutdown blocker.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8806368 [details]
Bug 1312406 and bug 1255361 - part 2: remove tab groups backup,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89826/diff/1-2/

Comment 8

[:Gijs (he/him)]

Comment on attachment 8806369 [details]
Bug 1312406 - part 3: bonus - remove dead icon,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89828/diff/1-2/

Comment 9

[Luís Miguel [:quicksaver] (not reading bugmail)]

Should the icon be removed in this patch from the svg only? I would think it should be removed from the pngs as well, or leave this one for consistency?

Note I don't use those icons so I don't really care either way, just a consistency-related thought that occurred to me. :)

Comment 10

[:Gijs (he/him)]

(In reply to Luís Miguel [:quicksaver] from comment #9)
> Should the icon be removed in this patch from the svg only? I would think it
> should be removed from the pngs as well, or leave this one for consistency?
> 
> Note I don't use those icons so I don't really care either way, just a
> consistency-related thought that occurred to me. :)

Removing from the png isn't really useful (unlikely it'll make any real size difference, and we'd need to update the coordinates of all the subsequent icons in the sprite which is really painful, and I'd need to image-edit all 15-odd Toolbar.png files which is then kind of a waste of time).

Comment 11

[Justin Dolske [:Dolske]]

Comment on attachment 8806367 [details]
Bug 1312406 - part 1: remove tab groups migration code,

https://reviewboard.mozilla.org/r/89824/#review89432

Comment 13

[Justin Dolske [:Dolske]]

Comment on attachment 8806368 [details]
Bug 1312406 and bug 1255361 - part 2: remove tab groups backup,

https://reviewboard.mozilla.org/r/89826/#review89434

Comment 14

[Justin Dolske [:Dolske]]

Comment on attachment 8806369 [details]
Bug 1312406 - part 3: bonus - remove dead icon,

https://reviewboard.mozilla.org/r/89828/#review89436

Comment 15

[Justin Dolske [:Dolske]]

*sigh* I reviewed those last two parts yesterday, but guess I didn't click the "Publish" button in ReviewBoard's confusing UI. :/ Sorry.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/465512fbb69b
part 1: remove tab groups migration code, r=Dolske
https://hg.mozilla.org/integration/autoland/rev/789a88b977b6
and bug 1255361 - part 2: remove tab groups backup, r=Dolske
https://hg.mozilla.org/integration/autoland/rev/66ab1149f151
part 3: bonus - remove dead icon, r=Dolske

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/465512fbb69b
https://hg.mozilla.org/mozilla-central/rev/789a88b977b6
https://hg.mozilla.org/mozilla-central/rev/66ab1149f151