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.

Sign up for GitHub

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

Jump to bottom

Fix javadocs to allow doc lint to pass in JDK 16 and above. #4834

Merged
merged 27 commits into from
Jan 18, 2023
Merged

Fix javadocs to allow doc lint to pass in JDK 16 and above. #4834

merged 27 commits into from
Jan 18, 2023

Conversation

usmansaleem
Copy link
Member

@usmansaleem usmansaleem commented Dec 16, 2022 •
edited
Loading

PR description

Adding missing javadocs so that javadoc doclint passes against JDK 16+ (invoke by Besu gradle build).

Exclude following packages from javadoc lint:

  • org.hyperledger.besu.privacy.contracts.generated
  • org.hyperledger.besu.tests.acceptance.*

Temporarily exclude ethereum and evm submodule for doc lint checks.

Important diffs:

Fixed Issue(s)
#2681

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

@usmansaleem
Fixing javadoc
28e94cf 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Generate javadoc for RocksDBStats
1a85464 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Generate javadoc for RocksDB plugin
b60c264 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Generate javadoc for kvstore service
898c835 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Generate javadoc for pipeline service
27a8975 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Generate javadoc for task service
1b0ddc3 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem marked this pull request as ready for review December 16, 2022 07:31
@usmansaleem
Updating plugin-api build.gradle with new hash as javadoc comments ca…
23e8282 
…used it to change

Signed-off-by: Usman Saleem <usman@usmans.info>
jflo
jflo approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@jflo jflo left a comment

Choose a reason for hiding this comment

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

Massive PR, I did not read every single javadoc, but the ones I spot checked are all reasonable. We don't expect these to be in-depth descriptions, but we can improve that over time.

Uzi you absolute legend, thanks for doing this thankless task.

@garyschulte
Copy link
Contributor

Uzi you absolute legend, thanks for doing this thankless task.

seconded. 👍

@siladu
Copy link
Contributor

siladu commented Dec 31, 2022

Kudos on this mega PR @usmansaleem! I've been playing trying to break the build by adding an un-javadoc'd method...

invoke by Besu gradle build

If you meant ./gradlew build then this isn't what I observed, I had to specifically run ./gradlew javadoc on java 16+ (I used java 17) to see errors.

It seems to be configured to run in circle CI (as part of the integration tests for some reason): https://github.com/hyperledger/besu/blob/main/.circleci/config.yml#L200-L203
however this won't break since circle ci is java 11.

So there's currently nothing to stop people committing non-compliant code onto main AFAICT.

Is there a way to force javadoc to throw errors even on java 11 so it will fail the build before we move to java 17?

@usmansaleem
Merge remote-tracking branch 'upstream/main' into javadoc_fix
38532d5 
Signed-off-by: Usman Saleem <usman@usmans.info>

# Conflicts:
#	besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java
#	besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java
#	besu/src/main/java/org/hyperledger/besu/controller/BesuController.java
#	besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java
#	config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java
#	config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java
#	config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java
#	ethereum/eth/src/main/java/org/hyperledger/besu/consensus/merge/ForkchoiceEvent.java
#	evm/src/main/java/org/hyperledger/besu/evm/EVM.java
#	evm/src/main/java/org/hyperledger/besu/evm/EvmSpecVersion.java
#	evm/src/main/java/org/hyperledger/besu/evm/MainnetEVMs.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/CodeFactory.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/CodeV0.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java
#	evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/CachedInvalidCodeRule.java
#	evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/ContractValidationRule.java
#	evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java
#	evm/src/main/java/org/hyperledger/besu/evm/gascalculator/ShanghaiGasCalculator.java
#	evm/src/main/java/org/hyperledger/besu/evm/operation/Operation.java
@usmansaleem
Adding missing javadoc after latest merge
3503a2d 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Copy link
Member Author

@siladu

Is there a way to force javadoc to throw errors even on java 11 so it will fail the build before we move to java 17?

It seems to me a javadoc/doclint has a bug in Java 11 which seems to be fixed post java 15. I was trying to see if there are any defaults which were disabled in 11 but enabled in Java 15+, but it doesn't seem so. What seems to be the plan to move to CircleCI build to Java 17, the current LTS?

@usmansaleem
Fixing code scanning alert - override clone method
48fa47c 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Run javadoc using Java 17 in CircleCI
17af349 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Run javadoc using Java 17 in CircleCI - medium class
ddcbaa0 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
test missing javadoc trigger build error in circleci
70f5e86 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
javadoc using github action
a6418b9 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
javadoc using github action
dd3ec91 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Revert "test missing javadoc trigger build error in circleci"
9d390d8 
This reverts commit 70f5e86.

Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Removing javadoc from circleci as it is moved to github actions
1ee35cf 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Removing javadoc from circleci as it is moved to github actions
6609c8b 
Signed-off-by: Usman Saleem <usman@usmans.info>
@siladu
Copy link
Contributor

siladu commented Jan 9, 2023

What seems to be the plan to move to CircleCI build to Java 17, the current LTS?

Last I heard we were planning to stay on CircleCI, rather than moving to github actions (which would have supported java 17).
I'm not sure of the java 17 status in general, I've asked on Discord. Hopefully it's just a case of updating CircleCI as you say 🤞

@usmansaleem
Merge remote-tracking branch 'upstream/main' into javadoc_fix
987333d 
Signed-off-by: Usman Saleem <usman@usmans.info>

# Conflicts:
#	evm/src/main/java/org/hyperledger/besu/evm/code/CodeV0.java
#	evm/src/main/java/org/hyperledger/besu/evm/operation/RetFOperation.java
@usmansaleem
Merge remote-tracking branch 'upstream/main' into javadoc_fix
6825dab 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Merge remote-tracking branch 'upstream/main' into javadoc_fix
b81df14 
Signed-off-by: Usman Saleem <usman@usmans.info>

# Conflicts:
#	consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadAttributes.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/CodeSection.java
#	evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java
#	evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java
#	evm/src/main/java/org/hyperledger/besu/evm/operation/Create2Operation.java
#	evm/src/main/java/org/hyperledger/besu/evm/operation/CreateOperation.java
#	plugin-api/build.gradle
@usmansaleem
Fix javadoc after merge conflicts
460cb44 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Update plugin-api known hash
b3a8d13 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Post merge conflicts - delete PayloadAttributes.java
c18fa6e 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
Merge remote-tracking branch 'upstream/main' into javadoc_fix
5fdcfcf 
Signed-off-by: Usman Saleem <usman@usmans.info>

# Conflicts:
#	consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java
#	consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java
#	consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java
#	plugin-api/build.gradle
#	plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionType.java
@usmansaleem
Fix javadoc after merge conflicts
5229e19 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem
plugin api checksum
dfd68a4 
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem self-assigned this Jan 18, 2023
@usmansaleem usmansaleem merged commit 9eb3283 into hyperledger:main Jan 18, 2023
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@usmansaleem @elenduuche
Fix javadocs to allow build to pass in JDK 17 (hyperledger#4834)
b0124de 
- Added missing javadocs so that javadoc doclint passes against JDK 17 (invoke by Besu gradle build).
- Exclude following packages from javadoc lint:
org.hyperledger.besu.privacy.contracts.generated
org.hyperledger.besu.tests.acceptance.*
- Temporarily exclude ethereum and evm submodule for doc lint checks.
- Run the javadoc task using GitHub actions (use Java 17) to report any javadoc errors during the PR builds
- Updating plugin-api build.gradle with new hash as javadoc comments caused it to change

Signed-off-by: Usman Saleem <usman@usmans.info>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
@usmansaleem @eum602
Fix javadocs to allow build to pass in JDK 17 (hyperledger#4834)
eab36da 
- Added missing javadocs so that javadoc doclint passes against JDK 17 (invoke by Besu gradle build).
- Exclude following packages from javadoc lint:
org.hyperledger.besu.privacy.contracts.generated
org.hyperledger.besu.tests.acceptance.*
- Temporarily exclude ethereum and evm submodule for doc lint checks.
- Run the javadoc task using GitHub actions (use Java 17) to report any javadoc errors during the PR builds
- Updating plugin-api build.gradle with new hash as javadoc comments caused it to change

Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem deleted the javadoc_fix branch March 13, 2024 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jflo jflo jflo approved these changes

Assignees

@usmansaleem usmansaleem

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@usmansaleem @garyschulte @siladu @jflo