GNOME Bugzilla – Bug 706232
accessibility.js in js/ui/status has style inconsistencies
Last modified: 2013-08-21 00:22:52 UTC
Created attachment 252098 [details] [review] fix a11y status style I spotted a couple of semi-colons that were missing from statements in the file which prompted me to run through and 'fix' a few other style inconsistencies I found. Although I am fairly sure adding semi-colons is a typo fix, I am less sure if the rest of the formatting was the right way to go about things and I welcome any comments on that. I have not been able to find the guidance stating what the maximum length of a line is meant to be, so if anyone can tell me that would be useful to make note of. Thanks.
Review of attachment 252098 [details] [review]: Looks good, thank you!
(In reply to comment #0) > Although I am fairly sure adding semi-colons is a typo fix, I am less sure if > the rest of the formatting was the right way to go about things and I welcome > any comments on that. In my opinion, we should ditch all those constants, but I'm probably alone on this. The fixes look good to me. > I have not been able to find the guidance stating what the maximum length of a > line is meant to be, so if anyone can tell me that would be useful to make note > of. We don't really have any guidance. I try to keep my lines to a decent length (under 100 chars) but we have code that goes way over that, sometimes written by me.
Thanks for the feedback both of you. I was not sure if adding the formatting was the right thing to do, so I am glad it has been well received. >In my opinion, we should ditch all those constants, Noted ;-). I guess I'll have to continue to try and make a judgement call on line lengths. I've been trying to keep it near to 80, but 100 seems more realistic until someone puts something in writing to suggest otherwise, I guess. Thanks again.
For the record, I think the constants are useful (they ensure all places get updated at once, if we need to change them), and I'd like them to stay :)
Attachment 252098 [details] pushed as d7cf203 - fix a11y status style