After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 692677 - layout: Merge Chrome and LayoutManager
layout: Merge Chrome and LayoutManager
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-01-28 06:54 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-01-28 17:41 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
layout: Shuffle code around (15.03 KB, patch)
2013-01-28 06:54 UTC, Jasper St. Pierre (not reading bugmail) 		committed Details | Review
layout: Rename some Chrome properties (6.46 KB, patch)
2013-01-28 06:54 UTC, Jasper St. Pierre (not reading bugmail) 		reviewed Details | Review
layout: Merge Chrome and LayoutManager (11.50 KB, patch)
2013-01-28 06:54 UTC, Jasper St. Pierre (not reading bugmail) 		committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-01-28 06:54:39 UTC 
This is a giant code cleanup that I've been meaning to do for a while.
Rationale in patch descriptions. This may not cleanly apply to master;
I can rebase if it fails.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-01-28 06:54:42 UTC 
Created attachment 234586 [details] [review]
layout: Shuffle code around

This will make the next diff much cleaner.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-01-28 06:54:44 UTC 
Created attachment 234587 [details] [review]
layout: Rename some Chrome properties

As we're planning on merging this with LayoutManager, switching
to use the LayoutManager names makes the next diff much cleaner.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-01-28 06:54:47 UTC 
Created attachment 234588 [details] [review]
layout: Merge Chrome and LayoutManager

The two classes have been gaining each other's functionality for a little
while, adding the new code wherever it was more convenient. Rather than
have a clear delineation between "This Manages Shell Chrome" and "This
Manages Shell Layout", I think it's better off if we just accept that
the responsibilities are pretty much the same.
Comment 4 Giovanni Campagna 2013-01-28 08:39:29 UTC 
Review of attachment 234586 [details] [review]:

Ok.
Comment 5 Giovanni Campagna 2013-01-28 08:40:17 UTC 
Review of attachment 234587 [details] [review]:

Nah, this one is not worth it, squash it with the next.
Comment 6 Giovanni Campagna 2013-01-28 08:41:14 UTC 
Review of attachment 234588 [details] [review]:

Yes.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-01-28 17:41:40 UTC 
you # Bug 692677 - layout: Merge Chrome and LayoutManager - NEW

Attachment 234586 [details] pushed as 72405cd - layout: Shuffle code around
Attachment 234588 [details] pushed as 8e231cb - layout: Merge Chrome and LayoutManager