JSHint fixes (including indentation & whitespace);#406
Conversation
jerone
added
CODE
|
Some of these fixes look like they are needed for consistency... however this is section blocked by #262 since I'm sure it deals with vote/flag/remove.... Placing on hold for the time being with a -1 until @sizzlemctwizzle can pipe in... there is still a lot of code normalizing to do, as well with strict mode too, but I've had to wait on that. |
|
@Martii commented on 6 nov. 2014 21:19 CET:
I understand your reasoning, where can I follow the current code of that issue? |
I can re-ask him in private if he doesn't mind being over trodden in that area but no promises. |
|
@Martii commented on 6 nov. 2014 21:33 CET:
Should have pinged him ( @sizzlemctwizzle ) myself, this private contacting is not a good way to work together and keep discussions public. |
|
He is in the public and so am I ... he just doesn't have a status update yet... sometimes everyone can use a reminder to go view a specific area... so non-issue there. |
|
I'm fairly opposed on these big formatting commits as they break code I'm too lazy to submit (not a good reason) and introduce bugs as there are no tests (not that I see any in this case). One rule I dislike about this commit has to be:
But now that I look at our styleguide, which I've totally read before, apparently the consistent Thanks for catching all the missing/extra I'll grumble and sigh when this is merged. Just be sure it's a quick clean cut. |
|
My personal opinion is to get this in as soon as possible; keep the base as consistent as possible and follow the styleguide as best possible, from the beginning. There we're no open PR's, so it looked the best time. |
|
I would like us to slowly actually follow And it's still "up in the air" about one liners although they should always have braces from my understanding of the STYLEGUIDE.md. JSHint isn't always correct either as proven multiple times in the past... but there are a few things that could be isolated from this PR, slowly, and merged in. |
There was a problem hiding this comment.
Why is the function body indented at the same level as the definition? Tabs vs Spaces issue?
There was a problem hiding this comment.
Because it wants function to be after the previous comma ,.
It's all good. Make sure to fix whatever the merge conflict your other patches made. |
…into jshint Conflicts: controllers/issue.js
|
@Zren commented on 6 nov. 2014 23:26 CET:
Done. |
|
There's definitely some issues with JSHint here... I'm against this merge but I will wait a bit to see if sizzle is available... JSHint should NEVER change TODO comment lines as it moves where a change should be done and there is also some compliance with our STYLEGUIDE that JSHint doesn't pay attention to... should know more later. |
That's not an JSHint issue. Can revert that change. Please point to the corresponding lines. |
As I stated above this is on HOLD regardless of styling because it's section blocked... I will get back to it at a later point... otherwise if you are impatient I'll just close it as invalid. |
3 participants
This PR includes:
No functionality should have been changed!
Node modules & ace has been ignored.
Some files still have warnings, but those warnings needs addressing later as it might involve functionality changes:
controllers/_template.jscontrollers/scriptStorage.jscontrollers/user.jslibs/modelQuery.jslibs/modifySessions.jslibs/muExpress.jslibs/remove.jslibs/templateHelpers.jsHoping that this can reviewed asap to keep conflicts as a minimum.