Skip to content

Conversation

kumar303
Copy link
Contributor

Fixes #204

@coveralls
Copy link

coveralls commented Jul 15, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 0dfe7a0 on kumar303:linter into b08de56 on mozilla:master.

src/cmd/index.js Outdated
import lint from './lint';
import sign from './sign';
export default {build, run, sign};
export default {build, run, sign, lint};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these are sorted with the exception of the addition.

@muffinresearch
Copy link
Contributor

I wonder if there would be a way to import the yargs config from the linter directly so you can expose the linter as a command and get all the cli features in the linter? Then as the linter changes you should just get the added functionality and it would help avoid any potential breakage should something in the linter change.

If this is working for now then it makes sense to not block on it, however, if there's anything that can be done on the linter side to make integration better/easier in future feel free to add issues there.

r+wc

@kumar303
Copy link
Contributor Author

kumar303 commented Jul 20, 2016

I wonder if there would be a way to import the yargs config from the linter directly

I thought about this too. The upside, as you mentioned, is that new linter program options would magically appear in web-ext. The downside is that it would be hard to know if any of those magic additions would make sense for web-ext or not. For example, it might break web-ext or a new option may be something that web-ext should handle directly (such as how it handles sourceDir).

So, yeah, I'm not entirely sure. Copying and pasting the configuration into web-ext makes it very explicit which is nice. If a new option gets added to linter without web-ext knowing about it, I envision someone filing a bug in web-ext saying "you don't support X." This would make it easy to fix.

@andymckay
Copy link
Contributor

200

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling fb5b974 on kumar303:linter into 30c0b4e on mozilla:master.

@kumar303 kumar303 merged commit 0b095aa into mozilla:master Jul 20, 2016
@kumar303 kumar303 deleted the linter branch July 20, 2016 19:22
kumar303 added a commit to kumar303/web-ext that referenced this pull request Jul 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants