Skip to content

Conversation

haydenbleasel
Copy link
Contributor

Typically Gulp plugins suppress the output of the native module and add an option to log the output. Basically:

.pipe(bless({
  log: true
}));

Gives the option of keeping the command line clean.

index.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't this just be if (options.log) {?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generally yes but for options I'm making it specific to avoid people getting true when entering 1, !0, !null, etc. Check this out for some interesting reading: http://dorey.github.io/JavaScript-Equality-Table/

Copy link
Member

Choose a reason for hiding this comment

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

!0 and !null are === true. Sure 1 isn't === true but why restrict it? Let's allow them to pass any "truthy" value to turn on logging.

@adam-lynch
Copy link
Member

The README would need to change. E.g. lines like this:

bless(options). The (optional) options argument is passed on as is to bless.js.

Maybe we should also say it defaults to false.

@adam-lynch
Copy link
Member

Do you know if this is testable?

@haydenbleasel
Copy link
Contributor Author

Not sure if it's unit testable but I have tested it manually. It's a fairly simple piece of code.

@adam-lynch
Copy link
Member

Yeah. Thanks.

adam-lynch added a commit that referenced this pull request Sep 17, 2014
Added option for Gulp logging
@adam-lynch adam-lynch merged commit 9617109 into BlessCSS:master Sep 17, 2014
@adam-lynch
Copy link
Member

Made a small change (8d39e6f) and published as 2.0.0.

@haydenbleasel
Copy link
Contributor Author

Yep awesome - no point building the message if we're not logging it. Thanks mate.

@adam-lynch
Copy link
Member

Nothing is foolproof. I managed to break it in 3.0.0. Just added tests, fixed it and published now as 3.0.1.

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.

2 participants