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.

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

Remove instrument(ClassReader) and analyzeClass(ClassReader) #850

Merged
merged 1 commit into from Mar 4, 2019

Conversation

Godin
Copy link
Member

@Godin Godin commented Mar 4, 2019

There are several problems with methods Instrumenter.instrument(ClassReader) and Analyzer.analyzeClass(ClassReader) :

So I think that we have only two options:

  1. mark them as deprecated and remove later
  2. remove them

@marchof WDYT?

@Godin Godin added this to the 0.8.4 milestone Mar 4, 2019
@Godin Godin self-assigned this Mar 4, 2019
@Godin Godin added this to Candidates in Current work items via automation Mar 4, 2019
@marchof
Copy link
Member

marchof commented Mar 4, 2019

@Godin Good point -> Remove

@Godin Godin changed the title Remove or deprecate Instrumenter.instrument(ClassReader) and Analyzer.analyzeClass(ClassReader) ? Remove instrument(ClassReader) and analyzeClass(ClassReader) Mar 4, 2019
@Godin Godin requested a review from marchof March 4, 2019 17:17
@Godin Godin moved this from Candidates to Review in Current work items Mar 4, 2019
@Godin
Copy link
Member Author

Godin commented Mar 4, 2019

For fun and future readers of this thread:

* they use field `ClassReader.b` which is marked as deprecated in ASM 7.1

* they don't work when field contains more than just bytes of one class
  (see `ClassReader(byte[] classFileBuffer, int classFileOffset, int classFileLength)`)
@marchof marchof merged commit faf49f9 into master Mar 4, 2019
Current work items automation moved this from Review to Done Mar 4, 2019
@marchof marchof deleted the issue-850 branch March 4, 2019 22:49
@Godin Godin mentioned this pull request Mar 5, 2019
@jacoco jacoco locked as resolved and limited conversation to collaborators May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants