Approve only single quotes - lint settings and improvemnts

[tradzik]

http://www.google-melange.com/gci/task/view/google/gci2014/5889353796550656

[namanyayg]

Hey @ty221, it seems we have three redundant commits from you at the top. Can they be merged into one? @RoonyH recently told me a trick with resetting to master then cherry-picking the latest commit. Try:

git reset --hard upstream/master git cherry-pick 9b435c2

then push with the --force flag.

Also, was the single quote lint part of any task or did any mentor tell you? Last I heard we had removed all checks. Finally, please do not modify any external files. Only the main.js file is checked by the lint, rest files are not and neither they need to be fixed.

[tradzik]

@namangoel1 @namanyayg

1. Thank you, sure I can rebase this commit, but... is that strongly necessary?
2. This is my task: http://www.google-melange.com/gci/task/view/google/gci2014/5889353796550656
3. I improved all files, exclude fully external library, there is nothing bad in that, I think.

[namangoel1]

1) Yes, you're including 3 commits. If it were one I think it still would've been okay. I strongly urge you to rebase or reset.

2) and 3) The task specifically says: 'Find all violations and fix those. Do not lint external files (libraries)'. I think @RoonyH could clarify here.

Also, I think it's wiser to have 'escape' set to true. Isn't the default value 'false' anyway?

[tradzik]

@namanyayg @namangoel1 If I improved something additionally - there is nothing bad in that. If you want I can set esacpe to true, however I think false is better there.

[namangoel1]

@ty221 Fair enough, as I said, let's wait for clarification from RoonyH.

Why do you think false is better?

[tradzik]

@namangoel1 @namanyayg I think it is really good, when we use only one kind of quote marks. If we have senctene "This's Mark", there is no problem to write it as 'This\'s Mark', and that is more clear

BTW - commands you gave do not work

[namangoel1]

@ty221 I think it's better to set escape to true. Escaping gets annoying if a sentence requires it multiple times. Let's wait for @RoonyH to clarify on both.

Btw, please tag only 'namangoel1', thanks!

[tradzik]

@namangoel1 I do not think it is annoying, just normal part of work, but... Ok, let's wait for @RoonyH

[roonyh]

Different people. Different opinions. Thanks both for your ideas. Since we need a final decision and Since i like that way, and according to github I am an owner :smile: I'll say lets go with escape true. This will allow people who only like single quotes can do that too. right? Its good if we all can follow the same style though.

And @ty221 good job! did you run the linters locally? Did you check if .jscs.json work by purposefully violating few rules? @namangoel1 can help you if you need.

Its better if we do not touch the external files. They are from fossasia domain but still they are not from our repo. Developers of those files will not follow our style guide so when we update versions of those external libraries our linters will blame us. It is simply an unnecessary thing to do. So please undo those changes.

And we have another js file here. don't worry about the one inside browser folder. And you need to add that to the gulp task. Currently its checking only main.js.

And dont worry much about the git commits.

[tradzik]

Hi @RoonyH ! Thank you for your comment! I will change escape option to true in the second. Answering your question, I ran jscs on my machine and basing on its result I fixed mistakes, as you can see.

I will add this one file to gulp config file.

I think now my work will satisfy your requirements now!

Please leave me a comment, if you will have any other suggestions!

---

PS I've undone these changes to external libraries, as you wanted. I will make new pull request for new changes

[tradzik]

Please take a look on continuation of this PR #214

@RoonyH @namangoel1