-
Notifications
You must be signed in to change notification settings - Fork 791
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
Muzzle improvements: docs, javadocs, renamings and minor refactoring #1379
Muzzle improvements: docs, javadocs, renamings and minor refactoring #1379
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
- 👍 Renamed almost all muzzle classes, moved some of them to two different packages (
collector
ormatcher
) depending on how and when they're used;- ❤️ Added/improved javadocs in classes that had no/poor documentation;
- ❤️ Documented compile-time and runtime muzzle functionality in
muzzle.md
;- ❤️ Documented muzzle gradle plugin usage (as much as I understand it);
- 👍 Split
Reference.Flag
to several different enums;- 👍 Refactored
HelperReferenceWrapper
so that it uses Java 8 streams instead of Guava.
docs/contributing/muzzle.md
Outdated
./gradlew :instrumentation:google-http-client-1.19:printReferences | ||
``` | ||
|
||
* `muzzle` task runs the runtime muzzle verification against different library versions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest putting muzzle
before printReferences
since most important
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to this PR, but should we rename the printReferences
to something that includes muzzle in the task name? e.g. muzzlePrintReferences
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pavolloffay makes sense 👍
...ng/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
Outdated
Show resolved
Hide resolved
...ng/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
Outdated
Show resolved
Hide resolved
...ng/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
Outdated
Show resolved
Hide resolved
...ng/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/HelperReferenceWrapper.java
Show resolved
Hide resolved
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
f411c3c
to
d700aee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Contents:
collector
ormatcher
) depending on how and when they're used;muzzle.md
;Reference.Flag
to several different enums;HelperReferenceWrapper
so that it uses Java 8 streams instead of Guava.This PR does not solve #536, but at least it clears up some of the questions that appeared there.