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

fix(@angular/cli): show errors after warnings #8745

Closed
wants to merge 1 commit into from

Conversation

Taha-Di-Nero
Copy link
Contributor

No description provided.

Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

Heya @Taha-Di-Nero, these changes look good, but I think we can get this in as a fix instead.

Could you rename the commit to fix(@angular/cli): show errors after warnings please?

@Taha-Di-Nero
Copy link
Contributor Author

Taha-Di-Nero commented Dec 7, 2017

Of course
@filipesilva Done

@Taha-Di-Nero Taha-Di-Nero changed the title feat(@angular/cli): Warning order #8292 fix(@angular/cli): show errors after warnings Dec 8, 2017
@Taha-Di-Nero
Copy link
Contributor Author

@filipesilva why merging is blocked ??

@filipesilva
Copy link
Contributor

Sorry it took awhile for me to come back to this PR. I've re-reviewed it and it should be ready. Thanks!

@clydin
Copy link
Member

clydin commented Jan 9, 2018

It looks like this is moving the warnings before the output stats. I'm not sure this is a good idea as this can cause any warnings to be easily missed. Warnings and errors are both diagnostic messages and I think it would make more sense to group them together as such. Also, errors are already shown after warnings. So the current order is information, warnings, errors. This represents an increasing level of severity and therefore shows the most urgent message last; which could be considered the order of most utility.

In regards to the actual underlying concern of too much text being displayed in a small console window, PR #8875 should provide partial relief. In addition, further work on reducing the verbosity of warnings and errors would also be a fruitful effort in this regard.

@filipesilva
Copy link
Contributor

To be honest I misunderstood the original issue (#8292). I thought warnings were actually showing after errors.

I agree with @clydin here: the warnings are already being shown before the errors so I don't think there's any need to change.

Apologies for that... I should have read read the issue and PR with more attention.

@filipesilva filipesilva closed this Jan 9, 2018
@Taha-Di-Nero Taha-Di-Nero deleted the fix-8292 branch January 10, 2018 10:09
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants