Skip to content

fix(@angular-devkit/build-angular): should not log duplicate messages #11628

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

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

Teamop
Copy link
Contributor

@Teamop Teamop commented Jul 23, 2018

Should not log duplicate messages for specFailure and onRunComplete, as they all have been handled by the baseReporter

@filipesilva
Copy link
Contributor

Hi, can you give me some context about the problem you are fixing? A way for me to see it happening would be important as well.

@Teamop
Copy link
Contributor Author

Teamop commented Jul 23, 2018

@filipesilva , yes it's for ng test, right now, all these reporters will log the same message twice, so here is just avoid the duplicate messages from these reporters, just leave to baseReporter to deal with them

@filipesilva
Copy link
Contributor

filipesilva commented Jul 23, 2018

I tried adding an error to an existing project, then introducing your changes, and I could still see duplicated error messages. Can you show me what do you see before and after your changes?

Edit: I noticed now that while there is still some duplication, it is less than before.

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.

Can you change the commit message to fix(@angular-devkit/build-angular): should not log duplicate messages please? (build_angular -> build-angular)

@@ -240,6 +240,9 @@ const eventReporter: any = function (this: any, baseReporterDecorator: any) {
failureCb && failureCb();
}
}

// avoid duplicate failure message
this.specFailure = (_browser: any, result: any) => {};
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be just this.specFailure = () => {};, and the same applies to the other ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, thanks

@Teamop
Copy link
Contributor Author

Teamop commented Jul 23, 2018

hi @filipesilva, right now, with these changes, you will still see some duplicate, as these is also another reporter progress which comes from karma, and I have also submitted a PR karma-runner/karma#3080

@Teamop Teamop changed the title fix(@angular-devkit/build_angular): should not log duplicate messages fix(@angular-devkit/build-angular): should not log duplicate messages Jul 23, 2018
@Teamop
Copy link
Contributor Author

Teamop commented Jul 23, 2018

@filipesilva could you restart all the CI tests? seems all of them aren't triggered

@Teamop
Copy link
Contributor Author

Teamop commented Jul 23, 2018

I will close and reopen to see if the CI can be restarted.

@Teamop Teamop closed this Jul 23, 2018
@Teamop Teamop reopened this Jul 23, 2018
@filipesilva
Copy link
Contributor

LGTM, thanks for this fix!

@filipesilva filipesilva merged commit adb5861 into angular:master Jul 24, 2018
@Teamop Teamop deleted the fix-duplicate branch July 24, 2018 10:28
@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.

3 participants