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

Option to disable warnings #488

Closed
mhartington opened this Issue Mar 14, 2017 · 16 comments

Comments

Projects
None yet
4 participants
@mhartington

mhartington commented Mar 14, 2017

Hi there!

We use sassdoc for the ionic source code and had a question regarding some warnings.

When it tried to parse a file without any doc content, we get warnings like this:

» [WARNING] SassDoc could not find anything to document.

  * Are you still using `/**` comments ? They're no more supported since 2.0.
    See <http://sassdoc.com/upgrading/#c-style-comments>.
  * Are you documenting actual Sass items (variables, functions, mixins, placeholders) ?
    SassDoc doesn't support documenting CSS selectors.
    See <http://sassdoc.com/frequently-asked-questions/#does-sassdoc-support-css-classes-and-ids->.

As not all of our sass files require documentation, we were wondering if it's possible to disable the warnings, as they tend to get pretty noisy.

I took a look at the docs and there doesn't seem to be an option for this, unless I missed it?

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 14, 2017

Interesting. I was pretty sure SassDoc was silent as a default, and loud with --verbose. Can you confirm you don’t use verbose? :)

@mhartington

This comment has been minimized.

mhartington commented Mar 14, 2017

Pretty sure we don't.

https://github.com/driftyco/ionic/blob/master/scripts/docs/processors/parse-sass.js#L145

This is part of a full build process, so we're pulling in package and calling it here.

@mhartington

This comment has been minimized.

mhartington commented Mar 14, 2017

Yeah just confirmed, that if we enable verbose in the options, we get bit more output.

» Folder `/Users/mhartington/GitHub/ionic/src/components/tabs/tabs.wp.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toast/toast.ios.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toast/toast.md.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toast/toast.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toast/toast.wp.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toggle/toggle.ios.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toggle/toggle.md.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/toggle/toggle.wp.scss` successfully parsed.
» Folder `/Users/mhartington/GitHub/ionic/src/components/virtual-scroll/virtual-scroll.scss` successfully parsed.
» [WARNING] SassDoc could not find anything to document.

It just looks like that warning is not ignored by the verbose setting.

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 14, 2017

It appears we don’t restrict this warning to the verbose option: https://github.com/SassDoc/sassdoc/blob/master/src/sassdoc.js#L346. Is there a reason behind it @pascalduez?

@pascalduez

This comment has been minimized.

Member

pascalduez commented Mar 15, 2017

This warning was intended to display if SassDoc could not find anything to document for the whole codebase of a project. In your setup though you run one instance of SassDoc per file, hence the warning. I'm not sure this is optimal actually, cause there's some elements being resolved at a codebase level, maybe vars in partials etc.
This check is supposed to happens at the end of the parsing phase.

We may introduce a new option, but that would need proper naming and docs.

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 18, 2017

We may introduce a new option, but that would need proper naming and docs.

I don’t want to introduce an option for that. Why can’t we restrict this warning to the verbose flag like the rest?

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 18, 2017

@mhartington Are we right to assume you fire an instance of SassDoc per file?

@pascalduez

This comment has been minimized.

Member

pascalduez commented Mar 18, 2017

I don’t want to introduce an option for that. Why can’t we restrict this warning to the verbose flag like the rest?

verbose is usualy meant at toggling additional or not critical informations I would say.
If your project didn't produced any documentation for some reasons (which can be seen as a fail), that's quite an important information, isn't it?
If some annotations parsing are failing because of an erroneous syntax, you might want to be aware of it.
We are heavily relying on this currently.

Crawling our code, the verbose flag is not used much actualy, only for logger.log not logger.warn.
And there are several places where we emit warnings, like in annotations etc.

That's why for the sake of not breaking nor refactoring the whole shit, I mentioned a new flag, although I agree we should limit their number at any costs.
Something like --silent? (aka I know what I'm doing) or --loglevel="..."

Or we can decide to put any kind of logging under the verbose flag defaulting to true, but that's a breaking change.

Just brainstorming here, no strong feelings. Maybe @valeriangalliat would have more ideas?

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 18, 2017

Something like --silent? (aka I know what I'm doing) or --loglevel="..."

I feel like loglevel would introduce too much complexity both on our side and the users’. I’d rather either make everything dependant on verbose, or make SassDoc verbose as by default and trade --verbose for --quiet.

@mhartington

This comment has been minimized.

mhartington commented Mar 20, 2017

@HugoGiraudel Sorry for the delay, yes we pass ever file to sass-doc as part of our build process.

@HugoGiraudel

This comment has been minimized.

Member

HugoGiraudel commented Mar 20, 2017

Why don’t you run a single instance of SassDoc for your entire codebase?

@mhartington

This comment has been minimized.

mhartington commented Mar 21, 2017

That would be more ideal, but build/documentation tasks processes each directory, one at a time. For example, take this page.

http://ionicframework.com/docs/v2/api/components/checkbox/Checkbox/

Not only is it processing the sass for here, it's also processing a typescript file with jsdoc annotations. For our setup, this works best for us.

pascalduez added a commit that referenced this issue Mar 21, 2017

Scope empty data message under the verbose flag
- This prevent unwanted logs for special configurations
- See #488

pascalduez added a commit that referenced this issue Mar 21, 2017

Scope empty data message under the verbose flag
- This prevent unwanted logs for special configurations
- See #488

pascalduez added a commit that referenced this issue Mar 21, 2017

Scope empty data message under the verbose flag
- This prevent unwanted logs for special configurations
- See #488
@pascalduez

This comment has been minimized.

Member

pascalduez commented Mar 23, 2017

@mhartington Please upgrade to sassdoc@2.2.1 you should not see the warning by default.

@pascalduez pascalduez closed this Mar 23, 2017

@mhartington

This comment has been minimized.

mhartington commented Mar 23, 2017

It looks as though 2.2.1 is not released to npm yet?

@pascalduez

This comment has been minimized.

Member

pascalduez commented Mar 24, 2017

@mhartington should be fixed now, sorry about that.

@jacksbox

This comment has been minimized.

jacksbox commented Aug 22, 2018

using sassdoc@2.5.0 I still see this warning by default.
In my usecase sassdoc runs twice, once with a custom template (which has a parser for the custom annotation) and once with the default template (which logs the [WARNING])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment