fix: Fix three small issues and transition codebase to SPDX identifiers #347

jsync-swirlds · 2025-01-10T00:09:59Z

full codebase

Replaced copyright notices with SPDX license identifiers
Added SPDX identifiers where needed
- In particular, the integration tests had almost no headers, so added to all files.
- Most of pbj-runtime and numerous other files also had no license header.

pbj integration tests

Added spotless plugin to the integration tests build
- Also ran a spotlessApply to fix many formatting issues.
Fixes Fix typo in integration tests #145
- Renamed intergration to integration globally

pbj core

Ran spotlessApply, which clearly hasn't been done in a very long time.
- Reverted changes because some files become unreadable and likely need
  format exceptions added.
Fixes PBJ Compiler fails on comment preceding reserved field or syntax keyword #217
- Added docComment before syntax, import, package, option,
  and reserved keywords in grammar
  - The root cause is the change to docComment to make it non-skipped, so
    we have to add it everywhere. Skipped comments (the // type) can be
    absolutely anywhere because the grammar skips them.
- Note, this will also partially address PBJ processes document comments (i.e. /**...*/) as tokens #319
- It is probable that more adjustments (particularly for comments at the end
  of onof blocks) will be needed.

github-actions · 2025-01-10T00:12:14Z

JUnit Test Report

0 files - 67 0 suites - 67 0s ⏱️ - 2m 39s
0 tests - 1 265 0 ✅ - 1 262 0 💤 - 3 0 ❌ ±0
0 runs - 7 120 0 ✅ - 7 101 0 💤 - 19 0 ❌ ±0

Results for commit f0fa610. ± Comparison against base commit c0fc03d.

♻️ This comment has been updated with latest results.

github-actions · 2025-01-10T00:19:34Z

Integration Test Report

399 files ±0 399 suites ±0 13m 38s ⏱️ + 3m 8s
115 027 tests ±0 115 027 ✅ ±0 0 💤 ±0 0 ❌ ±0
115 255 runs ±0 115 255 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 39efea5. ± Comparison against base commit e0ab4f8.

This pull request removes 103 and adds 103 tests. Note that renamed tests count towards both.

com.hedera.pbj.intergration.test.CompareToNegativeTest ‑ testNonComparableOneOfField()
com.hedera.pbj.intergration.test.CompareToNegativeTest ‑ testNonComparableSubObj()
com.hedera.pbj.intergration.test.CompareToNegativeTest ‑ testRepeatedField()
com.hedera.pbj.intergration.test.CompareToTest ‑ comareTo_unsigned()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_BoolValue()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_ByteValue()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_DoubleValue()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_FloatValue()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_Int32Value()
com.hedera.pbj.intergration.test.CompareToTest ‑ compareTo_Int64Value()
…

com.hedera.pbj.integration.test.CompareToNegativeTest ‑ testNonComparableOneOfField()
com.hedera.pbj.integration.test.CompareToNegativeTest ‑ testNonComparableSubObj()
com.hedera.pbj.integration.test.CompareToNegativeTest ‑ testRepeatedField()
com.hedera.pbj.integration.test.CompareToTest ‑ comareTo_unsigned()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_BoolValue()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_ByteValue()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_DoubleValue()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_FloatValue()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_Int32Value()
com.hedera.pbj.integration.test.CompareToTest ‑ compareTo_Int64Value()
…

♻️ This comment has been updated with latest results.

jsync-swirlds

I know this is a lot of files, but most are just adding SPDX headers, and the rest are mostly just spotless formatting or a renamed folder.
The actual bug fixes are in the Protobuf.g4 file and a couple comments in proto files to prevent regressions.

jasperpotts

Other than the grammar changes the should be changed to match what I committed already it all looks good :-) I hope the package renames are not going to be merge hell.

pbj-core/pbj-compiler/src/main/antlr/com/hedera/hashgraph/protoparser/grammar/Protobuf3.g4

jsync-swirlds · 2025-01-13T22:35:43Z

Other than the grammar changes the should be changed to match what I committed already it all looks good :-) I hope the package renames are not going to be merge hell.

I rebased in all the prior changes. The change to apply heiro configuration globally was harder (mostly because it forces spotless universally and documentation is minimal, so the choice is either do no quality checks or break the pbj-core layout, until we get the pbj-core code files annotated properly).

mishomihov00

Review applies to .github/workflows/zxc-compile-pbj-code.yaml

andrewb1269hg

Review and approve files assigned to devops-ci team.

* Fixes #145 * Renamed `intergration` to `integration` globally * Fixes #217 * Added documentation comments before `syntax`, `import`, `package`, `option`, and `reserved` keywords in grammar * The root cause is the change to docComment to make it non-skipped, so we have to add it everywhere. Skipped comments (the `//` type) can be absolutely anywhere because the grammar _skips_ them. * Note, this _might_ also fix #319 Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

* Replaced copyright notices with SPDX license identifiers * Added SPDX identifiers where needed * In particular, the integration tests had no headers, so added to all files. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

* Added spotless configurations * Also updated omnibus proto and antlr grammar * Adjusted grammar slightly and added some comments to the `everything.proto` test file * This adddresses some elements of processing reserved and oneof fields, but still cannot completely address docComments at the _end_ of a oneof block. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

* Removed license-header.txt to avoid spotless forcing the old header style back in. * Removed quality check requirement from pbj-core until the team has a chance to fix the problematic files. * There is no option to disable only spotless with the new plugin, so we're stuck with a heavy-handed approach here. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

* Removed extra doc comment entries for top-level items. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

jsync-swirlds self-assigned this Jan 10, 2025

jsync-swirlds force-pushed the change-to-spdx branch 7 times, most recently from 2f894c7 to 39efea5 Compare January 10, 2025 23:36

jsync-swirlds commented Jan 10, 2025

View reviewed changes

jsync-swirlds marked this pull request as ready for review January 10, 2025 23:57

jsync-swirlds requested review from a team as code owners January 10, 2025 23:57

jsync-swirlds requested review from dalvizu and mattp-swirldslabs January 10, 2025 23:57

rbair23 previously approved these changes Jan 11, 2025

View reviewed changes

jasperpotts reviewed Jan 11, 2025

View reviewed changes

pbj-core/pbj-compiler/src/main/antlr/com/hedera/hashgraph/protoparser/grammar/Protobuf3.g4 Show resolved Hide resolved

jsync-swirlds dismissed rbair23’s stale review via e0eacab January 13, 2025 20:55

jsync-swirlds force-pushed the change-to-spdx branch 7 times, most recently from 0fe7dae to ae1a5f6 Compare January 13, 2025 22:31

jsync-swirlds requested a review from jasperpotts January 13, 2025 22:37

mishomihov00 previously approved these changes Jan 14, 2025

View reviewed changes

andrewb1269hg previously approved these changes Jan 14, 2025

View reviewed changes

jsync-swirlds added 5 commits January 14, 2025 11:55

Change to SPDX identifiers

3d797ef

* Replaced copyright notices with SPDX license identifiers * Added SPDX identifiers where needed * In particular, the integration tests had no headers, so added to all files. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

Code review comments

f0fa610

* Removed extra doc comment entries for top-level items. Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>

jsync-swirlds dismissed stale reviews from andrewb1269hg and mishomihov00 via f0fa610 January 14, 2025 18:57

jsync-swirlds force-pushed the change-to-spdx branch from ae1a5f6 to f0fa610 Compare January 14, 2025 18:57

jsync-swirlds requested review from mishomihov00, andrewb1269hg and rbair23 January 14, 2025 18:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix three small issues and transition codebase to SPDX identifiers #347

fix: Fix three small issues and transition codebase to SPDX identifiers #347

jsync-swirlds commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

jsync-swirlds left a comment •

edited

Loading

jasperpotts left a comment

jsync-swirlds commented Jan 13, 2025 •

edited

Loading

mishomihov00 left a comment

andrewb1269hg left a comment

fix: Fix three small issues and transition codebase to SPDX identifiers #347

Are you sure you want to change the base?

fix: Fix three small issues and transition codebase to SPDX identifiers #347

Conversation

jsync-swirlds commented Jan 10, 2025 • edited Loading

full codebase

pbj integration tests

pbj core

github-actions bot commented Jan 10, 2025 • edited Loading

JUnit Test Report

github-actions bot commented Jan 10, 2025 • edited Loading

Integration Test Report

jsync-swirlds left a comment • edited Loading

Choose a reason for hiding this comment

jasperpotts left a comment

Choose a reason for hiding this comment

jsync-swirlds commented Jan 13, 2025 • edited Loading

mishomihov00 left a comment

Choose a reason for hiding this comment

andrewb1269hg left a comment

Choose a reason for hiding this comment

jsync-swirlds commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

jsync-swirlds left a comment •

edited

Loading

jsync-swirlds commented Jan 13, 2025 •

edited

Loading