search

dequelabs / axe-firefox-devtools

Integration of axe-core into the Firefox developer tools
17 stars 6 forks source link

Devtools eslint #36

Closed eeejay closed 8 years ago

CLAassistant commented 8 years ago

CLA assistant check
All committers have signed the CLA.

eeejay commented 8 years ago

This fixes issue #35

dylanb commented 8 years ago

@eeejay 2 issues

We would prefer to keep the tabs rather than spaces for the current files. The coding guideline seems to allow either tabs or spaces but not mixed. Can you change all whitespace to tabs.

Also, before merging, we will need the CLA signed.

eeejay commented 8 years ago

A quick grep in the browser's devtools source shows that they don't use tabs in JS. The only exception are upstream files that are pulled in externally.

dylanb commented 8 years ago

Not being allowed is different from not being used. From the eslint setup, it looks like its allowed. If that is the case, it would result in fewer changed lines which means we could keep more of our blame history.

eeejay commented 8 years ago

It is not allowed. Both the devtools style guide and the Mozilla one prohibit it. https://wiki.mozilla.org/DevTools/CodingStandards#Code_style https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style

As for git blame, that is why this change needs to happen earlier rather than later.

On Tue, May 24, 2016 at 11:16 AM Dylan Barrell notifications@github.com wrote:

Not being allowed is different from not being used. From the eslint setup, it looks like its allowed. If that is the case, it would result in fewer changed lines which means we could keep more of our blame history.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/dequelabs/axe-firefox-devtools/pull/36#issuecomment-221351685