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.

Sign up for GitHub

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

Jump to bottom

[NEMO-369] DirectByteArrayOutputStream usage refactoring #206

Merged
merged 4 commits into from
Mar 26, 2019
Merged

[NEMO-369] DirectByteArrayOutputStream usage refactoring #206

merged 4 commits into from
Mar 26, 2019

Conversation

hy00nc
Copy link
Member

@hy00nc hy00nc commented Mar 25, 2019 •
edited
Loading

JIRA: NEMO-369: DirectByteArrayOutputStream usage refactoring

Major changes:

  • Replaced usage of duplicated method getCount() to size()
  • deleted getCount() method

Minor changes to note:

  • None

Tests for the changes:

  • None

Other comments:

  • None

@wonook
Copy link
Member

wonook commented Mar 25, 2019

Please read the contributing guide and follow the PR process. Thanks!

@hy00nc hy00nc changed the title Code refactoring [NEMO-369] DirectByteArrayOutputStream usage refactoring Mar 25, 2019
DifferentSC
DifferentSC requested changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@DifferentSC DifferentSC left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your work, @hy00nc.

runtime/executor/src/main/java/org/apache/nemo/runtime/executor/data/DataUtil.java Outdated Show resolved Hide resolved
DifferentSC
DifferentSC previously approved these changes Mar 25, 2019
View reviewed changes
@DifferentSC
Copy link
Contributor

For me this PR seems okay. @sanha could you take a look?

DifferentSC
DifferentSC previously approved these changes Mar 26, 2019
View reviewed changes
Copy link
Contributor

@DifferentSC DifferentSC left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your work, @hy00nc!

@hy00nc
Copy link
Member Author

hy00nc commented Mar 26, 2019

@sanha @DifferentSC Thank you for the reviews!

@DifferentSC
Copy link
Contributor

There are check style errors. Please fix them before I can merge this PR.

@DifferentSC DifferentSC self-requested a review March 26, 2019 13:33
DifferentSC
DifferentSC approved these changes Mar 26, 2019
View reviewed changes
Copy link
Contributor

@DifferentSC DifferentSC left a comment

Choose a reason for hiding this comment

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

LGTM and passed CI. Great Job, @hy00nc!

@DifferentSC DifferentSC merged commit 62fb70f into apache:master Mar 26, 2019
@hy00nc hy00nc deleted the nemo_refactor branch April 10, 2019 05:16
wonook added a commit to snuspl/incubator-nemo that referenced this pull request Apr 30, 2019
@wonook
revert apache#206
5f39686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanha sanha sanha left review comments

@DifferentSC DifferentSC DifferentSC approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hy00nc @wonook @DifferentSC @sanha