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 672926 - history-service: assorted cleanups
history-service: assorted cleanups
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-03-27 15:02 UTC by Claudio Saavedra
Modified: 2012-03-28 21:48 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
history-service: remove unused variables (1.43 KB, patch)
2012-03-27 15:02 UTC, Claudio Saavedra 		committed Details | Review
history-service: remove unnecessary type cast (972 bytes, patch)
2012-03-27 15:02 UTC, Claudio Saavedra 		committed Details | Review
history-service: simplify sort function (870 bytes, patch)
2012-03-27 15:02 UTC, Claudio Saavedra 		rejected Details | Review

Description Claudio Saavedra 2012-03-27 15:02:48 UTC 
These patches fix a few things I found while reading the code.
Comment 1 Claudio Saavedra 2012-03-27 15:02:50 UTC 
Created attachment 210702 [details] [review]
history-service: remove unused variables
Comment 2 Claudio Saavedra 2012-03-27 15:02:53 UTC 
Created attachment 210703 [details] [review]
history-service: remove unnecessary type cast
Comment 3 Claudio Saavedra 2012-03-27 15:02:56 UTC 
Created attachment 210704 [details] [review]
history-service: simplify sort function
Comment 4 Xan Lopez 2012-03-27 22:48:15 UTC 
Review of attachment 210702 [details] [review]:

OK.
Comment 5 Xan Lopez 2012-03-27 22:48:36 UTC 
Review of attachment 210703 [details] [review]:

Ok.
Comment 6 Xan Lopez 2012-03-27 22:50:52 UTC 
Review of attachment 210704 [details] [review]:

I don't think it's a goal for this code to be as short as possible. Do you think this is easier to read?
Comment 7 Claudio Saavedra 2012-03-28 05:20:10 UTC 
(In reply to comment #6)
> Review of attachment 210704 [details] [review]:
> 
> I don't think it's a goal for this code to be as short as possible. Do you
> think this is easier to read?

Absolutely.
Comment 8 Xan Lopez 2012-03-28 15:52:13 UTC 
(In reply to comment #7)
> > I don't think it's a goal for this code to be as short as possible. Do you
> > think this is easier to read?
> 
> Absolutely.

Maybe I'm an old-timer, but for me compare methods usually follow that style, so it's easier to figure what's going on if they are like that. For instance, this is how the example in the GNOME doc does an example for the sort method of a queue:

7 return (id1 > id2 ? +1 : id1 == id2 ? 0 : -1); 

Anyway, no big deal, so I say let's leave it like it is.
Comment 9 Claudio Saavedra 2012-03-28 21:48:01 UTC 
Attachment 210702 [details] pushed as ce80dce - history-service: remove unused variables
Attachment 210703 [details] pushed as aaa40d7 - history-service: remove unnecessary type cast