Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Execution without options #986

Closed
markelog opened this issue Feb 1, 2015 · 18 comments
Closed

Execution without options #986

markelog opened this issue Feb 1, 2015 · 18 comments

Comments

@markelog
Copy link
Member

markelog commented Feb 1, 2015

If project doesn't have jscs config and If jscs cli tool is executed without options but with path argument - jscs <files>. What should we do?

Two possible ways:

  1. Apply rules from "default" preset, like with go fmt, standart npm package or with python pep8 tool. While proposing to use auto-config option at the end of error list.
  2. Enable auto-config option by default.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@markelog markelog added the cli label Feb 1, 2015
@markelog
Copy link
Member Author

markelog commented Feb 1, 2015

/cc @mdevils, @mrjoelkemp, @mikesherov, @zxqfox

@qfox
Copy link
Member

qfox commented Feb 1, 2015

I think auto-config by default is not a good idea. You can easily mistype files or forget to put paths and you'll get wrong config.

3 . Tell user that there are no rules to check and give him a note how to configure with preset or autoconfigure jscs.

But (1) is pretty fine too.

@mdevils
Copy link
Member

mdevils commented Feb 1, 2015

I like the current behaviour.

@hzoo
Copy link
Member

hzoo commented Jun 7, 2015

Also 👎 for the default preset for me. We could make the error message with no config better like explaining autoconfigure or using the preset flag.

ESLint has many default rules setup and they are actually planning on moving to have no rules turned on by default (many complaints about what should be in by default, etc). They just added a feature to extend other people's configs (can publish eslint-config-x) http://eslint.org/docs/developer-guide/shareable-configs which is pretty cool (like our presets, except it's by the user rather than ESLint itself).

@markelog
Copy link
Member Author

markelog commented Jun 8, 2015

That is because eslint added and removed default rules with each new version, which is irritating, we good here, because we have strict semver policy, so in our case, if we would add or remove rules that cause more errors, we would do that only in major.

@markelog
Copy link
Member Author

markelog commented Jun 8, 2015

Defining your own config is hard, even if you define only online like "preset": ..., since you would need to know the rules of that preset or at least have basic idea of that guide.

But if we would use default preset, people wouldn't need to think about that stuff, they just would start working, this is main advantage of the https://github.com/feross/standard package, this is main reason why people used it, i think we should listen the community and plan accordingly

@mrjoelkemp
Copy link
Member

Defining your own config is hard, even if you define only online like "preset": ..., since you would need to know the rules of that preset or at least have basic idea of that guide.

+1, though the barrier to entry is high only for preset authors. We have the presets at the top of the Overview section on the website, so we've at least attempted to make it easy to get started.

What would the default preset be? I'd vote for airbnb as it seems to be the most mainstream.

@mikesherov
Copy link
Contributor

I think this problem is solved by adding an --init option, which is simply --auto-configure . followed by --fix . and perhaps even automatically electing (f)ix for every violation.

Because we exclude node_modules by default (although auto-configure should probably also read your gitignore for excluded files), the result is a fixed up codebase with a simple jscsrc that contains only the preset chosen by the user.

Our docs should have a big hero banner at the top:
npm install -g jscs && jscs --init

I don't think we need to promote a default preset, for the reasons outlined already. It creates unneeded holy wars for very little value for the user.

@hzoo
Copy link
Member

hzoo commented Jul 7, 2015

Our docs should have a big hero banner at the top:
npm install -g jscs && jscs --init

I would like that to be on the main homepage too - related to #1286 as well.

@markelog
Copy link
Member Author

I think this problem is solved by adding an --init option, which is simply --auto-configure . followed by --fix . and perhaps even automatically electing (f)ix for every violation.

See #1471

Our docs should have a big hero banner at the top:
npm install -g jscs && jscs --init

Well, some users don't need that, for example they might base their decision of the preset on how good it is, not on amount of errors current code is actually has, plus autofixing deals with most errors anyway - basically it doesn't matter how many errors it finds autofix will work up most of them. Also what if there no code yet, what if user chose the jscs config without any code?

It creates unneeded holy wars for very little value for the user.

Similar decision was chosen by the eslint maintainers for the default presets, that it would create holly wars, but it didn't happened in jscs.

I think users are a lot more friendly then we might think they are :-).

I'm hoping 3.0 will be out in next couple months, we basically already started working on it - #1502.

So i think for this couple of months we might try this, if it wouldn't go well, we will revert in 3.0.

What would the default preset be? I'd vote for airbnb as it seems to be the most mainstream.

This is most popular preset according to github traffic, so i would say yes.

@mikesherov
Copy link
Contributor

Some users might not need it but those users typically need docs less. The easier it is to get started using it, the better. I still vote for an init option as the easiest path towards using JSCS.

The auto fix and auto configure options are what seem to blow people's minds and are the quickest easier path to a fixed up codebase. Yes, people could base their decisions on many things, but giving them an init that makes them not have to think about it is the most user friendly.

Again, we could have a default preset in auto configure with zero controversy. Once we choose a default, it will make users mad no matter what we do to it afterwards.

So i think for this couple of months we might try this, if it wouldn't go well, we will revert in 3.0.

Have users been asking for a default preset? I see zero evidence this is a thing people want.

@mrjoelkemp
Copy link
Member

Users aren't explicitly asking for it, but as Oleg said, the success of
feross/standard shows an implicit desire.
On Jul 12, 2015 6:23 AM, "Mike Sherov" notifications@github.com wrote:

Some users might not need it but those users typically need docs less. The
easier it is to get started using it, the better. I still vote for an init
option as the easiest path towards using JSCS.

The auto fix and auto configure options are what seem to blow people's
minds and are the quickest easier path to a fixed up codebase. Yes, people
could base their decisions on many things, but giving them an init that
makes them not have to think about it is the most user friendly.

Again, we could have a default preset in auto configure with zero
controversy. Once we choose a default, it will make users mad no matter
what we do to it afterwards.

So i think for this couple of months we might try this, if it wouldn't go
well, we will revert in 3.0.

Have users been asking for a default preset? I see zero evidence this is a
thing people want.


Reply to this email directly or view it on GitHub
#986 (comment).

@mikesherov
Copy link
Contributor

I say let feross/standard take on that maintenance burden then. Most linters that start becoming opinionated later revert.

Simply because a popular library exists doesn't mean we need to copy their functionality. Why aren't we content being a module consumed by standard and allow them to plug along as a standard bearer while we continue to focus on what made us popular to begin with?

Consider the sublime text plugin... suddenly it's going to start throwing errors when you clone a repo that doesn't have a .jscsrc? And those errors are for a style guide you didn't choose or ask for? Blecch. 


Sent from Mailbox

On Sun, Jul 12, 2015 at 12:27 PM, Joel Kemp notifications@github.com
wrote:

Users aren't explicitly asking for it, but as Oleg said, the success of
feross/standard shows an implicit desire.
On Jul 12, 2015 6:23 AM, "Mike Sherov" notifications@github.com wrote:

Some users might not need it but those users typically need docs less. The
easier it is to get started using it, the better. I still vote for an init
option as the easiest path towards using JSCS.

The auto fix and auto configure options are what seem to blow people's
minds and are the quickest easier path to a fixed up codebase. Yes, people
could base their decisions on many things, but giving them an init that
makes them not have to think about it is the most user friendly.

Again, we could have a default preset in auto configure with zero
controversy. Once we choose a default, it will make users mad no matter
what we do to it afterwards.

So i think for this couple of months we might try this, if it wouldn't go
well, we will revert in 3.0.

Have users been asking for a default preset? I see zero evidence this is a
thing people want.


Reply to this email directly or view it on GitHub
#986 (comment).


Reply to this email directly or view it on GitHub:
#986 (comment)

markelog added a commit to markelog/node-jscs that referenced this issue Jul 13, 2015
markelog added a commit to markelog/node-jscs that referenced this issue Jul 13, 2015
@markelog
Copy link
Member Author

Most linters that start becoming opinionated later revert.

JSLint, ESLint, JSHint all has default configs they didn't revert that behaviour. I'm not sure what are you referring to.

Simply because a popular library exists doesn't mean we need to copy their functionality.

Python, Go and many more has that "default config", whereas in more mature languages static analises tools with "default config" included in the compiler itself. We actually also received couple issues that "jscs doesn't lint" simple because user didn't set their own jscs config.

And again, popularity of standart also shows that, some people don't want to set their own config, they don't care about style, for them, it doesn't matter what some config looks like, it just needed to exist.

Why aren't we content being a module consumed...

This fix wouldn't change that, it doesn't mean we would became opinionated, we would have default as most tools do, a convention, that you could change if you want to.

Consider the sublime text plugin... suddenly it's going to start throwing errors when you clone a repo that doesn't have a .jscsrc? And those errors are for a style guide you didn't choose or ask for? Blecch.

That is why this is a major change, but that actually not gonna happen, for that case plugin would work as it did before - https://github.com/SublimeLinter/SublimeLinter-jscs/blob/aff99a9be7685a2c1eef7288d899a79520c100f4/linter.py#L35

If some plugins do not do that, they can adapt, again, this is why this is a major change, one way or another plugins would need to adapt. For example we no longer show "There is no errors..." or our exit codes now completely different, those tools could depend on that, but there is no going forward, without breaking a few eggs.

Even if they don't use jscs --config=.jscsrc, "One code style to rule them all" seems as a good future to me. For example, most of the Python, Go, Ruby written programs use one code style. Why should JS be any different? If it shouldn't be, then ppl would simple use their own config, that would be a way to tell us they don't want that.

Everything i stated above should be a big pointer for us, i think we shouldn't ignore all that.

This is might be a bad decision, yes, good intention and road to hell, all that, but it also might be a very good one, we don't know, right now, we have opinions, couple pointers and no data.

Since next major version (let's Hope!) should come up soon enough, that would be a good experimental period.

markelog added a commit to markelog/node-jscs that referenced this issue Jul 20, 2015
@lahmatiy
Copy link
Contributor

@mdevils I like the current behaviour.

I agree with Marat, current behaviour is better choice. Explicit better than implicit here. When no config found tool should warn about it, but doesn't silently check against default code style. Last one may be really confusing, especially if something wrong with your config location.

@qfox
Copy link
Member

qfox commented Jul 25, 2015

Well, It's pretty hard, because personally I like the idea with some very common default preset. But seems like airbnb is not the best choice. It has esnext, it has some specific things (like "_this" for context, requiring padding before line comments, etc). But yes, this preset the most common of existent.

And actually, the more I think about it the less I like it. Here is comparison of presets: https://gist.github.com/zxqfox/12994393bd9b7681cbab

Users aren't explicitly asking for it

JSLint, ESLint, JSHint all has default configs

But it's not about code style. It's about syntax and implicit things that are generating errors or so.

Python, Go and many more has that "default config"

Python is a bad example, indentation there is a syntax. So they more like require it. Just hard to write it right.

Not sure why Go has it. But feels like it reduces the barrier to entry for newcomers, and makes community more focused on good things. So It's a always a good choice (btw that's why I like an idea with using the most common preset to check the code).


For now I'm voting 👎 because of:

  • esnext: true — we shouldn't use experimental things as default behaviour, imho;
  • checking with some specific rules (that airbnb has) can confuse user more than just silent exit (take a look at comparison);
  • If we go that way when we could change the default behaviour? ;-(

Btw. I still think we can help user to configure jscs if there are no config after an error.

@hzoo
Copy link
Member

hzoo commented Oct 3, 2015

Lets orphan this as well? I think the majority of us disagree on doing this at the moment anyway

@markelog
Copy link
Member Author

markelog commented Oct 5, 2015

Resolution of this ticket depends on #1064

@hzoo hzoo closed this as completed Feb 26, 2016
@hzoo hzoo added the revisit label Feb 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants