Improve field handling #212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Marcono1234
commented
Aug 19, 2022
Comment on lines
+46
to
+50
| <dependency> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>animal-sniffer-annotations</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
Is there a reason why animal-sniffer-annotations was previously not a dependency? It allows directly referring to the annotation type IgnoreJRERequirement in SignatureChecker.
Though if you want I can also revert this again.
olamy
requested changes
Aug 31, 2022
copybara-service bot
pushed a commit
to google/guava
that referenced
this pull request
Apr 24, 2023
The main motivation to this CL is to pick up [a fix to Animal Sniffer](mojohaus/animal-sniffer#212): The old version of Animal Sniffer ignores suppression annotations when it looks at fields, and that was causing me problems as I tried to use a field of type `FileAttribute`. While I was at it, I ran the usual command for updating deps (but not plugins), as documented in cl/503506407. And then, I inlined a couple properties into the `dependencyManagement` entries that used them. We'd introduced the properties because we used to use multiple artifacts that were released together. However, we don't use multiple artifacts with those properties anymore: - We stopped using the Animal Sniffer _annotations_ way back in cl/273964192, leaving only the Animal Sniffer plugin. - We stopped using the Checker Framework's `compat-qual` annotations back in cl/399471446 - cl/399480336, leaving only the main `qual` annotations. - But we had temporarily introduced a usage of the `sources` artifact for `qual` in cl/364918297. We removed the usage of it in cl/399190627 but left behind the `dependencyManagement` entry. So this CL removes that, too. (It's possible that we'd want to move away from using properties for versions in general: While Guava doesn't use Dependabot for `pom.xml` updates (only for GitHub Actions updates, which are easily for our tools to import), we can still update all deps by running a command. So we're not stuck hand-editing multiple locations, and thus all the property provides a layer of indirection. However, until we have a way to automatically update _plugins_, there is some sense in using properties for _plugin_ versions. Or at least there's sense in it for any properties that we still need to use in multiple places. But we might not need to use many (any?) in multiple places anymore, since we finally solved the mystery of why our `pluginManagement` section wasn't being respected back in cl/492304151....) RELNOTES=n/a PiperOrigin-RevId: 526467147
copybara-service bot
pushed a commit
to google/guava
that referenced
this pull request
Apr 24, 2023
The main motivation to this CL is to pick up [a fix to Animal Sniffer](mojohaus/animal-sniffer#212): The old version of Animal Sniffer ignores suppression annotations when it looks at fields, and that was causing me problems as I tried to use a field of type `FileAttribute`. While I was at it, I ran the usual command for updating deps (but not plugins), as documented in cl/503506407. And then, I inlined a couple properties into the `dependencyManagement` entries that used them. We'd introduced the properties because we used to use multiple artifacts that were released together. However, we don't use multiple artifacts with those properties anymore: - We stopped using the Animal Sniffer _annotations_ way back in cl/273964192, leaving only the Animal Sniffer plugin. - We stopped using the Checker Framework's `compat-qual` annotations back in cl/399471446 - cl/399480336, leaving only the main `qual` annotations. - But we had temporarily introduced a usage of the `sources` artifact for `qual` in cl/364918297. We removed the usage of it in cl/399190627 but left behind the `dependencyManagement` entry. So this CL removes that, too. (It's possible that we'd want to move away from using properties for versions in general: While Guava doesn't use Dependabot for `pom.xml` updates (only for GitHub Actions updates, which are easily for our tools to import), we can still update all deps by running a command. So we're not stuck hand-editing multiple locations, and thus all the property provides a layer of indirection. However, until we have a way to automatically update _plugins_, there is some sense in using properties for _plugin_ versions. Or at least there's sense in it for any properties that we still need to use in multiple places. But we might not need to use many (any?) in multiple places anymore, since we finally solved the mystery of why our `pluginManagement` section wasn't being respected back in cl/492304151....) RELNOTES=n/a PiperOrigin-RevId: 526651811
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ElementType.FIELDas possible target toIgnoreJRERequirementFeedback is appreciated! If you want I can also split the commits into separate pull requests.