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

Update logging to be less verbose #454

Closed
wants to merge 1 commit into from
Closed

Update logging to be less verbose #454

wants to merge 1 commit into from

Conversation

eddiemonge
Copy link
Contributor

Instead of showing all the files that have changed, state how many have
changed (per change type) unless verbose is passed. Then display all the
changes.

@eddiemonge
Copy link
Contributor Author

@vladikoff the failing tests are already existing failures. taking a look at fixing them for a separate commit.

@XhmikosR
Copy link
Member

@eddiemonge: please fetch and rebase.

@eddiemonge
Copy link
Contributor Author

@XhmikosR updated

test.expect(2);
var cwd = path.resolve(fixtures, 'patterns');
var assertWatch = helper.assertTask('watch', {cwd: cwd});
var assertWatch = helper.assertTask('watch', {cwd:cwd, verbose:true});
Copy link
Member

Choose a reason for hiding this comment

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

The spaces after ":" are needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry, I thought your changes removed them.

Copy link
Member

Choose a reason for hiding this comment

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

It's actually the grunt preset in JSCS that requires them.

Instead of showing all the files that have changed, state how many have
changed (per change type) unless verbose is passed. Then display all the
changes.
@eddiemonge
Copy link
Contributor Author

updated again

@XhmikosR
Copy link
Member

I'm not sure I'm completely 100% with this. I mean, I see there are some cases it might make sense but if it's a couple of files passing verbose might be too much.

/CC @vladikoff for feedback.

@eddiemonge
Copy link
Contributor Author

verbose should be just that, verbose. non-verbose should be concise. In the attached example, other tasks do minimal output by default. This one doesn't; its excessively noisy

files

@XhmikosR
Copy link
Member

Like I said I see the use case for this.

It's just that we shouldn't overdo it like with other contrib plug-ins
where no output is shown at all.

Anyway just wait for others to chime in.
On Oct 14, 2015 20:49, "Eddie Monge" notifications@github.com wrote:

verbose should be just that, verbose. non-verbose should be concise. In
the attached example, other tasks do minimal output by default. This one
doesn't; its excessively noisy

[image: files]
https://cloud.githubusercontent.com/assets/127535/10492480/3bef6668-7261-11e5-870e-5d08bf7448b5.png


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

@eddiemonge
Copy link
Contributor Author

It's just that we shouldn't overdo it like with other contrib plug-ins where no output is shown at all.

I agree which is why I send pull requests to those to show something by default and more when asked. Just because something else is broken shouldn't mean don't fix something else.

@trkoch
Copy link

trkoch commented Oct 27, 2015

You may also want to take a look at #467. It doesn't change any defaults and is merely an opt-in feature. The previous output can always be shown when run with --verbose.

Personally I like to see no output at all (when no errors occur), because usually what I care about are the tasks triggered.

@eddiemonge
Copy link
Contributor Author

closing as this is over 3 years old and unlikely to land even though it would be nice to have

@eddiemonge eddiemonge closed this Jul 25, 2018
@eddiemonge eddiemonge deleted the logging branch July 25, 2018 16:11
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.

None yet

3 participants