Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

[hadoop] remove duplicate diagnostics #2527

Merged
merged 2 commits into from
Apr 10, 2019

Conversation

mzmssg
Copy link
Member

@mzmssg mzmssg commented Apr 10, 2019

More readable patch: https://github.com/mzmssg/hadoop/pull/4/files
There will no duplicate stderr in diagnostics:
Original
image

Fixed
image

@mzmssg mzmssg requested a review from yqwang-ms April 10, 2019 08:27
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.932% when pulling bf49219 on zimiao/remove_duplicate_diagnostics into 07e4e5f on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.932% when pulling bf49219 on zimiao/remove_duplicate_diagnostics into 07e4e5f on master.

@yqwang-ms
Copy link
Member

Can we just move line 1575 to 1579 and 1596?
It seems more nature.
image

@mzmssg
Copy link
Member Author

mzmssg commented Apr 10, 2019

It's the same. But we need three if (exitEvent.getDiagnosticInfo() != null) in different block, and split diagnostic and its prefix Diagnostic message from attempt in different blocks.
I don't think it's more readable.

Copy link
Member

@yqwang-ms yqwang-ms left a comment

Choose a reason for hiding this comment

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

It should be different style, I perfer functional style, anyway it is fine.

@mzmssg mzmssg merged commit 72db13b into master Apr 10, 2019
@mzmssg mzmssg added this to the Mid April Release milestone Apr 12, 2019
@mzmssg mzmssg deleted the zimiao/remove_duplicate_diagnostics branch May 24, 2019 06:29
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

3 participants