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 764155 - display: Remove unused variable
display: Remove unused variable
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Display
unspecified
Other All
: Normal normal
: ---
Assigned To: Debarshi Ray
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-24 16:02 UTC by Bastien Nocera
Modified: 2016-07-14 12:55 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
display: Remove unused variable (1.37 KB, patch)
2016-03-24 16:02 UTC, Bastien Nocera 		accepted-commit_now Details | Review
display: Ensure only one output is set as primary (1.82 KB, patch)
2016-03-24 18:42 UTC, Rui Matos 		committed Details | Review

Description Bastien Nocera 2016-03-24 16:02:34 UTC 
.
Comment 1 Bastien Nocera 2016-03-24 16:02:39 UTC 
Created attachment 324691 [details] [review]
display: Remove unused variable
Comment 2 Bastien Nocera 2016-03-24 16:08:40 UTC 
I think we might want to spelunk a little to check what it was used for, and whether it should really be removed.
Comment 3 Rui Matos 2016-03-24 16:11:22 UTC 
Review of attachment 324691 [details] [review]:

++
Comment 4 Bastien Nocera 2016-03-24 16:12:00 UTC 
(In reply to Rui Matos from comment #3)
> Review of attachment 324691 [details] [review] [review]:
> 
> ++

Did you check what it was used for?
Comment 5 Rui Matos 2016-03-24 18:42:17 UTC 
(In reply to Bastien Nocera from comment #4)
> Did you check what it was used for?

Now I have. It seems like it was used to break out of the loop and avoid setting all outputs as primary but it doesn't seem like it was correct.

Dave Airlie removed it in https://git.gnome.org/browse/gnome-control-center/commit/?id=0005c32ded0f3e81963493d4bcf9be9a9d2844fc but this logic is still wrong. I'm attaching a patch to fix it that can go on top of that variable removal.
Comment 6 Rui Matos 2016-03-24 18:42:32 UTC 
Created attachment 324717 [details] [review]
display: Ensure only one output is set as primary

When switching an output to secondary we'd set all other outputs as
primary. Instead, set only the first we find.
Comment 7 Bastien Nocera 2016-04-15 09:54:32 UTC 
Review of attachment 324717 [details] [review]:

That looks fine to me, but hard to test without the right hardware.
Comment 8 Rui Matos 2016-05-02 16:26:41 UTC 
Comment on attachment 324717 [details] [review]
display: Ensure only one output is set as primary

Attachment 324717 [details] pushed as 6d825b9 - display: Ensure only one output is set as primary
Comment 9 Rui Matos 2016-07-14 12:55:46 UTC 
this should have been closed