-
Notifications
You must be signed in to change notification settings - Fork 864
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
Fix Gradle 7.0 compatibility issues and build warnings #288
Conversation
Removes as many Gradle 7.0 compatibility issues as possible * `baseName` -> `archiveBaseName` * `extension` -> `archiveExtension` * `destinationDir` -> `destinationDirectory` * `runtime` -> `runtimeOnly` * Change some log4j-api and log4j-core dependencies * Remove an unneeded and outdated plugin (`net.ltgt.apt`) * tweak the plugin-api change detector's property annotations. Warnings still exist with one external plugin used for license file checking that we do not control the soruce code for. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
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.
LGTM
'Implementation-Version': calculateVersion() | ||
) | ||
} | ||
} | ||
|
||
dependencies { | ||
api 'org.apache.logging.log4j:log4j-api' |
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.
Interesting, is the reason for promoting from implementation
to api
due to a Log4j-api class being in one of the signatures of the util
classes, or Gradle 7.0?
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.
They are getting rid of the runtime
dependency type, and runtimeOnly
excludes the dependent types from compilation visibility. i.e. runtime
was api
+ runtimeOnly
. This seemed cleaner than adding log4j2 dependencies in 10 or so new build files.
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.
This seemed cleaner than adding log4j2 dependencies in 10 or so new build files.
True.
Log4j-api
is used in pretty much every submodule (due to the fine amount of Besu logging) and could be considered a kind of common dependency 🤔
Being unique in that respect, I can see the value in having as an api
dependency 👍
Removes as many Gradle 7.0 compatibility issues as possible * `baseName` -> `archiveBaseName` * `extension` -> `archiveExtension` * `destinationDir` -> `destinationDirectory` * `runtime` -> `runtimeOnly` * Change some log4j-api and log4j-core dependencies * Remove an unneeded and outdated plugin (`net.ltgt.apt`) * tweak the plugin-api change detector's property annotations. Warnings still exist with one external plugin used for license file checking that we do not control the source code for. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Removes as many Gradle 7.0 compatibility issues as possible * `baseName` -> `archiveBaseName` * `extension` -> `archiveExtension` * `destinationDir` -> `destinationDirectory` * `runtime` -> `runtimeOnly` * Change some log4j-api and log4j-core dependencies * Remove an unneeded and outdated plugin (`net.ltgt.apt`) * tweak the plugin-api change detector's property annotations. Warnings still exist with one external plugin used for license file checking that we do not control the source code for. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com> Signed-off-by: edwardmack <ed@edwardmack.com>
Removes as many Gradle 7.0 compatibility issues as possible
baseName
->archiveBaseName
extension
->archiveExtension
destinationDir
->destinationDirectory
runtime
->runtimeOnly
net.ltgt.apt
)Warnings still exist with one external plugin used for license file
checking that we do not control the soruce code for.
Signed-off-by: Danno Ferrin danno.ferrin@gmail.com