Skip to content

[BEAM-3845] Remove deprecated Class.newInstance() method usage#9613

Merged
iemejia merged 1 commit intoapache:masterfrom
lgajowy:BEAM-3845
Oct 7, 2019
Merged

[BEAM-3845] Remove deprecated Class.newInstance() method usage#9613
iemejia merged 1 commit intoapache:masterfrom
lgajowy:BEAM-3845

Conversation

@lgajowy
Copy link
Contributor

@lgajowy lgajowy commented Sep 19, 2019

Class.newInstance() is deprecated since Java 9. See more verbose description on Jira.

I searched for all Class.newInstance() usages in Beam's codebase and It seems that this was the only one usage of Class.newInstance() in beam codebase left. :) Other places that require the new instance use either Constructor.newInstance() or Class.getDeclaredConstructor().newInstance(). Both replacements are acceptable and are not deprecated in Java 9.

@iemejia could you take a look?


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- Build Status
XLang --- --- --- Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@lgajowy lgajowy requested a review from iemejia September 19, 2019 13:23
@iemejia
Copy link
Member

iemejia commented Sep 23, 2019

Run Python PreCommit

@lgajowy
Copy link
Contributor Author

lgajowy commented Sep 24, 2019

@alanmyrvold and @yifanzou: we noticed together with @iemejia that this class (TestScripts) is used only by the quickstart-java-*.groovy, starter-generation.groovy and mobilegaming-java-*.groovy scripts. The scripts (quickstart, mobilegaming...) in turn seem to be not used by any job or gradle task in Beam's repo.
Therefore, I have a question: are the groovy scripts still used anywhere or will be used? Or is it a "dead code" and we should delete this?

@alanmyrvold
Copy link
Member

alanmyrvold commented Sep 24, 2019 via email

@lgajowy
Copy link
Contributor Author

lgajowy commented Sep 25, 2019

@alanmyrvold right - now I see clearly that it is invoked from the generated task: https://github.com/apache/beam/blob/master/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L1575

Thanks!

cc: @iemejia

@lgajowy
Copy link
Contributor Author

lgajowy commented Oct 4, 2019

@iemejia does this PR LGTY? :)

Copy link
Member

@iemejia iemejia left a comment

Choose a reason for hiding this comment

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

LGTM
Please take care of the JIRA issue since I am not sure if it is already ready to be marked as Fixed/Closed.

@iemejia iemejia merged commit 5a7bd66 into apache:master Oct 7, 2019
@lgajowy
Copy link
Contributor Author

lgajowy commented Oct 7, 2019

Thanks, issue resolved. 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants