Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: lint for dangling commas #5091

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 4, 2016

Lint for dangling commas in the final lines of object/array declarations.

The no-dangling-comma-on-final-line instances (over 4000) in the code base vastly outnumber the dangling comma instances (less than 80, mostly in tests). So, for consistency, convert those ~80 instances to the dominant practice.

@Trott Trott added the tools Issues and PRs related to the tools directory. label Feb 4, 2016
@Trott
Copy link
Member Author

Trott commented Feb 4, 2016

@Fishrock123
Copy link
Contributor

Why?

It's actually potentially less churn into the future to enforce dangling commas in object literals since that means you never have to edit the previous land line.

@bnoordhuis
Copy link
Member

I'm with Jeremiah. I like dangling commas, they keep diff noise down.

@Fishrock123
Copy link
Contributor

Mind you since the no-comma style apparently that style has 4000 instances that probably isn't viable. I move we just don't do anything honestly.

@bnoordhuis
Copy link
Member

Right, I'm not saying we update everything in one fell swoop. It would be nice if we could enforce it for new code, though.

@Trott
Copy link
Member Author

Trott commented Feb 5, 2016

@Fishrock123 @bnoordhuis I too am a fan of the dangling comma. I'm just a bigger fan of consistent coding conventions enforced by tooling. But yeah, you're not wrong, for sure.

@Trott Trott closed this Feb 6, 2016
@rvagg
Copy link
Member

rvagg commented Feb 8, 2016

oo! let's go wholesale comma-first, that's one I can get behind and it defiantly reduces diff noise!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants