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

[SPARK-24420][Build][FOLLOW-UP] Upgrade ASM6 APIs #22082

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@gatorsmile
Member

gatorsmile commented Aug 12, 2018

What changes were proposed in this pull request?

Use ASM 6 APIs after we upgrading it to ASM6.

How was this patch tested?

N/A

@gatorsmile

This comment has been minimized.

Show comment
Hide comment
@gatorsmile
Member

gatorsmile commented Aug 12, 2018

@kiszk

This comment has been minimized.

Show comment
Hide comment
@kiszk

kiszk Aug 12, 2018

Contributor

LGTM

Contributor

kiszk commented Aug 12, 2018

LGTM

@SparkQA

This comment has been minimized.

Show comment
Hide comment
@SparkQA

SparkQA Aug 12, 2018

Test build #94639 has finished for PR 22082 at commit 2666500.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

SparkQA commented Aug 12, 2018

Test build #94639 has finished for PR 22082 at commit 2666500.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.
@HyukjinKwon

This comment has been minimized.

Show comment
Hide comment
@HyukjinKwon

HyukjinKwon Aug 12, 2018

Member

retest this please

Member

HyukjinKwon commented Aug 12, 2018

retest this please

@srowen

srowen approved these changes Aug 12, 2018

The change just updates code to actually use ASM6 in these places? OK, pending tests.

@SparkQA

This comment has been minimized.

Show comment
Hide comment
@SparkQA

SparkQA Aug 12, 2018

Test build #94643 has finished for PR 22082 at commit 2666500.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

SparkQA commented Aug 12, 2018

Test build #94643 has finished for PR 22082 at commit 2666500.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
@dbtsai

This comment has been minimized.

Show comment
Hide comment
@dbtsai

dbtsai Aug 12, 2018

Member

retest this please

Member

dbtsai commented Aug 12, 2018

retest this please

@dbtsai

This comment has been minimized.

Show comment
Hide comment
@dbtsai

dbtsai Aug 12, 2018

Member

LGTM. @gatorsmile out of my curiosity, did you run into any issue that asm6 generates older version of bytecode defined in asm5?

Member

dbtsai commented Aug 12, 2018

LGTM. @gatorsmile out of my curiosity, did you run into any issue that asm6 generates older version of bytecode defined in asm5?

@HyukjinKwon

This comment has been minimized.

Show comment
Hide comment
@HyukjinKwon

HyukjinKwon Aug 13, 2018

Member

retest this please

Member

HyukjinKwon commented Aug 13, 2018

retest this please

@SparkQA

This comment has been minimized.

Show comment
Hide comment
@SparkQA

SparkQA Aug 13, 2018

Test build #94660 has finished for PR 22082 at commit 2666500.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

SparkQA commented Aug 13, 2018

Test build #94660 has finished for PR 22082 at commit 2666500.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
@dbtsai

This comment has been minimized.

Show comment
Hide comment
@dbtsai

dbtsai Aug 13, 2018

Member

Merged into master. Thanks.

Member

dbtsai commented Aug 13, 2018

Merged into master. Thanks.

@asfgit asfgit closed this in a992827 Aug 13, 2018

@gatorsmile

This comment has been minimized.

Show comment
Hide comment
@gatorsmile

gatorsmile Aug 14, 2018

Member

@dbtsai Nope. I did not hit any issue. :)

Member

gatorsmile commented Aug 14, 2018

@dbtsai Nope. I did not hit any issue. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment