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

BREAKING: Remove built-in copy of standard #17

Merged
merged 3 commits into from
Apr 7, 2017
Merged

Conversation

feross
Copy link
Member

@feross feross commented Apr 7, 2017

standard is no longer bundled with snazzy. You must install standard manually alongside snazzy.

Run npm install standard --save-dev to get a copy of standard, then run standard | snazzy where you previously used to ran snazzy.

Though this takes more steps, it's better because it allows the user to control the exact version of standard that gets used. And for users who were piping into snazzy all along, this means a quicker install, since an extra copy of standard will not get installed.

@feross feross force-pushed the remove-standard branch 4 times, most recently from 5f1a654 to ac7e149 Compare April 7, 2017 01:47
`standard` is no longer bundled with snazzy. You must install `standard` manually alongside `snazzy`.

Run `npm install standard --save-dev` to get a copy of `standard`, then run `standard | snazzy` where you previously used to ran `snazzy`.

Though this takes more steps, it's better because it allows the user to control the exact version of `standard` that gets used. And for users who were piping into `snazzy` all along, this means a quicker install, since an extra copy of `standard` will not get installed.
bin/cmd.js Outdated
}
})

process.stdout.on('error', function () {})

Choose a reason for hiding this comment

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

is this line intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that I introduced that line to prevent a bug in Node.js, but I can't find the commit where I introduced it. This line was been in the codebase since the initial commit.

I'll remove it and see if anything breaks! 😅

@ungoldman
Copy link

@feross just one comment, otherwise looks good. Also might want to bump node versions in .travis.yml, still targeting 0.10/0.12/iojs from back in the day.

@jsumners
Copy link

jsumners commented Apr 7, 2017

What if standard read some sort of output config from package.json like:

{
  "standardjs": {
    "formatter": "snazzy"
  }
}

And then standard did something like:

var outputFormatter
try {
  outputFormatter = require(moduleNameFromConfig)
} catch (e) {
  outputFormatter = normalDefaultOutput
}

@feross
Copy link
Member Author

feross commented Apr 7, 2017

Thanks for the review, @ungoldman!

@jsumners I'm hesitant to add config options to standard when doing standard | snazzy will suffice. This is more unixy.

@feross feross merged commit 296f03c into master Apr 7, 2017
@feross feross deleted the remove-standard branch April 7, 2017 18:41
@feross
Copy link
Member Author

feross commented Apr 7, 2017

7.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants