QRun-IO/qbit-sftp-data-integration
3
0 Fork 0
Code Issues Pull Requests 2 Actions Packages Projects Releases 3 Wiki Activity

build: re-pin to qbit-build-parent 2.0.0 and back-merge main (QRun-IO/qqq#770) #4

Merged
KofTwentyTwo merged 13 commits from feature/GH-770-repin-build-parent-2 into develop 2026-09-26 06:08:55 +00:00
Conversation 1 Commits 13 Files Changed 12
+528 -884
KofTwentyTwo commented 2026-09-26 01:53:46 +00:00 (Migrated from github.com)
Copy link
Copy source

Summary

Re-pins qbit-sftp-data-integration to com.kingsrook:qbit-build-parent:2.0.0 (qqq 4.0.0 GA, Java 21) and adds an opt-in qqq-snapshot profile that verifies it on qqq 4.1.0-SNAPSHOT, so qqq-all can include it. The first commit back-merges main into develop, because main had 7 commits develop lacked. Develop was on parent 1.4.0 (qqq 0.27.9, Java 17), so this is the largest jump of the qbit re-pins.

Merge this PR with a merge commit, not squash or rebase. A squash would drop the back-merge's link to main, and main would then look diverged from develop again.

Changes

  • 133e22b chore: merge main into develop (git merge --no-ff origin/main). Brings in main's Java 21 parent 1.5.1, the 0.3.0 release, orb 0.5.0, the Apache-2.0 LICENSE and NOTICE, .github/release.yml, and the standardized README. Develop keeps its CODEOWNERS and CLAUDE.md. Conflict resolutions are in the next section.
  • 5bb811b test: cover missing cron time zone in config customizers. Adds testCronExpressionWithoutTimeZoneIsRejected to both SyncSFTPExportConfigScheduledJobTransformStepTest and SyncSFTPImportConfigScheduledJobTransformStepTest. They cover the error branch of the time zone lookup that the next commit migrates: a cron with no time zone on insert, and a sparse update whose old record has no time zone either. The existing testInsertThenUpdateTwice already covers the old-record fallback. I added the tests on parent 1.5.1 before the re-pin, and they passed there.
  • 3bb034c build: re-pin to qbit-build-parent 2.0.0 (qqq 4.0.0). Parent 1.5.1 to 2.0.0. Two qqq 4.0 API removals broke the build. Each is migrated with the smallest change:
    • BREAK-04-18, main source, SFTPExportConfigCustomizer and SFTPImportConfigCustomizer: the removed generic getValueFromRecordOrOldRecord becomes ValueUtils.getValueAsString(getValueFromRecordElseFromOldRecord(...)). The lookup is the same. The only difference is that a non-String value is now converted instead of throwing ClassCastException.
    • BREAK-04-11, test only, BaseTest: QInstance.setAuthentication(...) becomes withInstanceDefaultAuthentication(...).
    • The README prerequisite now says "QQQ application (v4.0.0+)". It said v0.35.0+.
  • 76755fe build: add opt-in qqq-snapshot profile. It imports qqq-bom-pom:${qqq.snapshot.version} (default 4.1.0-SNAPSHOT) ahead of the parent's BOM and adds the Central snapshots repository. The parent has no qqq-version property to override. Default builds take the qqq version only from the parent. This profile matches the one in the B3 re-pin (qbit-customizable-table-views#7).
  • 57ffb1c ci: keep package installs unattended on CircleCI. Adds the configure-package-install command from qbit-quick-search develop and runs it as a pre-steps entry on all 5 jobs. It stops install_java from hanging on the needrestart "Daemons using outdated libraries" prompt (QRun-IO/qqq-orb#11). The command block is identical to quick-search's, which I checked with yq. The indentation follows this file's 2-space style.
  • fe32798 docs: refresh repo notes for the parent 2.0.0 re-pin. Replaces the stale "develop and main have diverged / two compile breaks" cautions in CLAUDE.md with how the qqq version is set and checked now.

The other plan items needed no change. The child pom had no qqq-bom-pom import shadowing the parent, and it pins no explicit qqq module <version>s (qqq-backend-core and qqq-backend-module-filesystem are versionless). There are no Docker-backed tests: all 12 tests use the in-memory and local filesystem backends, so the "fail, not skip, when CI=true" rule has nothing to change. Under CI=true, 0 tests are skipped.

Merge conflict resolutions

File develop main Resolution
LICENSE (add/add) AGPL-3.0 with a QRun-IO preamble (4670ba6, 2025-12-25) Apache-2.0 (7fd2506, 2026-01-05) Took main. It is the owner's later decision on the released branch, and it matches main's NOTICE, which merged in cleanly. Keeping develop's AGPL text beside an Apache NOTICE would contradict itself. I wrote no new license text. The pom <licenses>, file headers and checkstyle header template are untouched and still say AGPL; aligning them is the separate owner decision the plan calls for. Please confirm this choice in review.
.circleci/config.yml kingsrook/qqq-orb@0.3.8 kingsrook/qqq-orb@0.5.0 (Java 21 PR #2) Took main. 0.5.0 is the orb that installs Java 21. I did not bump it further (quick-search is on 0.5.2).
pom.xml parent qbit-build-parent 1.4.0 (Java 17) 1.5.1 (Java 21) Took main. It is the newer Java 21 parent. 3bb034c then moves it to 2.0.0.
pom.xml <properties> Moved the block to the top, revision 0.2.1 Left it below <scm>, revision 0.3.0 Kept develop's layout with main's value, 0.3.0, so there is one <properties> block. 0.3.0 is already released, and develop's 0.2.1 is older. On develop, the orb's calculate_version.sh turns a stable version into the next minor snapshot, so develop publishes 0.4.0-SNAPSHOT. It does not re-publish 0.3.0. I did not pick the next release number.

Everything else merged cleanly: NOTICE, .github/release.yml and README.md came from main, and CODEOWNERS and CLAUDE.md came from develop. The merge commit itself built green (row 1 below).

Related Issues

Closes QRun-IO/qqq#770
Part of QRun-IO/qqq-all#1

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected): consumers now get qqq 4.0.0 transitively
  • Documentation update
  • Refactoring (no functional changes)
  • Dependencies update

Testing

I ran every command from the repo root with Maven 3.9.16 and OpenJDK 21.0.12.1. There is no mvnw, so I used mvn. I never ran mvn install.

# Command Result
1 mvn -B clean verify on the merge result (parent 1.5.1) BUILD SUCCESS, 10 tests, qqq 0.35.0
2 mvn -B clean verify with the new tests (parent 1.5.1) BUILD SUCCESS, 12 tests
3 mvn -B test -Dtest='SyncSFTP*ConfigScheduledJobTransformStepTest' with the time zone check disabled in both customizers (temporary, reverted) 2 failures, both in testCronExpressionWithoutTimeZoneIsRejected (expected: <1> but was: <0>)
4 Same command with the old-record map replaced by Optional.empty() (temporary, reverted) 2 failures, both in testInsertThenUpdateTwice (expected: <1 1 1 * * ?> but was: <0 0 0 * * ?>)
5 Parent set to 2.0.0, then mvn -B clean verify BUILD FAILURE: compile error, getValueFromRecordOrOldRecord not found in both customizers
6 After the main-source migration, mvn -B clean verify BUILD FAILURE: test compile error, QInstance.setAuthentication not found in BaseTest
7 After the test migration, mvn -B clean verify BUILD SUCCESS, 12 tests, qqq 4.0.0
8 Before the profile, mvn -B dependency:list -Pqqq-snapshot -DincludeGroupIds=com.kingsrook.qqq The requested profile "qqq-snapshot" could not be activated, resolves 4.0.0 (red)
9 After the profile, same command qqq-backend-core and qqq-backend-module-filesystem 4.1.0-SNAPSHOT. Without -Pqqq-snapshot: 4.0.0
10 mvn -B dependency:list -Pqqq-snapshot -Dqqq.snapshot.version=0.35.0 -DincludeGroupIds=com.kingsrook.qqq 0.35.0, so the version override works

Final gates on HEAD fe32798:

Gate Command Result
qqq 4.0.0 (parent) CI=true mvn -B clean verify BUILD SUCCESS, 12 tests, 0 failures, 0 errors, 0 skipped
qqq 4.1.0-SNAPSHOT CI=true mvn -B clean verify -Pqqq-snapshot BUILD SUCCESS, 12 tests, 0 failures, 0 errors, 0 skipped
qqq 4.1.0-SNAPSHOT, published artifacts only CI=true mvn -B -U clean verify -Pqqq-snapshot -Dmaven.repo.local=/tmp/m2-b8 (empty local repo) BUILD SUCCESS, 12 tests, 0 failures. Resolved qqq-backend-core-4.1.0-20260926.013614-9 from sonatype-snapshots
Java 21 javap -v on a compiled class class file major version 65

I also ran CI=true mvn -B verify and CI=true mvn -B verify -Pqqq-snapshot without clean: both BUILD SUCCESS, 12 tests. I added the isolated-repo gate because the shared ~/.m2 can hold locally installed qqq snapshots; it shows the build passes against the published Sonatype snapshot.

CircleCI config: circleci config validate and the CircleCI config helper both stop at "Cannot find kingsrook/qqq-orb@0.5.0 in the orb registry". They report the same thing for develop's untouched config (orb 0.3.8), so it is a private-orb lookup limit and not a config error. yq parses the file, and all 5 jobs list the configure-package-install pre-step. On this PR, CircleCI qqq-orb/mvn_test_only (build 32) passed in 1m21s, so the orb resolves and install_java no longer hangs on the needrestart prompt.

  • Unit tests pass (mvn test)
  • Integration tests pass (mvn verify)
  • Manual testing performed

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas (no new logic; the profile and CI step carry comments)
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings (see the parent warning below)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Screenshots (if applicable)

Not applicable.

Additional Notes

  • Merge with a merge commit (see the Summary).
  • Parent warning, not fixed here: every build logs [WARNING] Failed to build parent project for com.kingsrook.qbits:qbit-sftp-data-integration:jar:0.3.0. It already appeared on parent 1.5.1 in this repo. The published parent pom declares its <version> as the literal ${revision}. The effective model is still correct (qqq 4.0.0, Java 21). The fix belongs in qbit-bom.
  • JUnit stays on 5.10.0: the child pom imports junit-bom:5.10.0, which wins over the parent's junit-bom:6.0.1. Tests run on JUnit 5.10.0. I left it alone because the plan only asks to remove shadowing qqq-bom-pom imports.
  • Out of scope, left as is: the pom <licenses>, the Java source headers, the pom header comment and checkstyle/license.txt (all still AGPL; a separate owner decision), the Kingsrook URLs in <url> and <scm>, the orb version beyond main's 0.5.0, and the open Dependabot alert for the test-scoped assertj-core 3.27.3 (the existing Dependabot branch covers it).
  • Not merged. This needs review first.
## Summary Re-pins `qbit-sftp-data-integration` to `com.kingsrook:qbit-build-parent:2.0.0` (qqq 4.0.0 GA, Java 21) and adds an opt-in `qqq-snapshot` profile that verifies it on qqq `4.1.0-SNAPSHOT`, so qqq-all can include it. The first commit back-merges `main` into `develop`, because `main` had 7 commits `develop` lacked. Develop was on parent 1.4.0 (qqq 0.27.9, Java 17), so this is the largest jump of the qbit re-pins. **Merge this PR with a merge commit, not squash or rebase.** A squash would drop the back-merge's link to `main`, and `main` would then look diverged from `develop` again. ## Changes - `133e22b` **chore: merge main into develop** (`git merge --no-ff origin/main`). Brings in main's Java 21 parent 1.5.1, the 0.3.0 release, orb 0.5.0, the Apache-2.0 `LICENSE` and `NOTICE`, `.github/release.yml`, and the standardized README. Develop keeps its `CODEOWNERS` and `CLAUDE.md`. Conflict resolutions are in the next section. - `5bb811b` **test: cover missing cron time zone in config customizers**. Adds `testCronExpressionWithoutTimeZoneIsRejected` to both `SyncSFTPExportConfigScheduledJobTransformStepTest` and `SyncSFTPImportConfigScheduledJobTransformStepTest`. They cover the error branch of the time zone lookup that the next commit migrates: a cron with no time zone on insert, and a sparse update whose old record has no time zone either. The existing `testInsertThenUpdateTwice` already covers the old-record fallback. I added the tests on parent 1.5.1 before the re-pin, and they passed there. - `3bb034c` **build: re-pin to qbit-build-parent 2.0.0 (qqq 4.0.0)**. Parent 1.5.1 to 2.0.0. Two qqq 4.0 API removals broke the build. Each is migrated with the smallest change: - BREAK-04-18, main source, `SFTPExportConfigCustomizer` and `SFTPImportConfigCustomizer`: the removed generic `getValueFromRecordOrOldRecord` becomes `ValueUtils.getValueAsString(getValueFromRecordElseFromOldRecord(...))`. The lookup is the same. The only difference is that a non-String value is now converted instead of throwing `ClassCastException`. - BREAK-04-11, test only, `BaseTest`: `QInstance.setAuthentication(...)` becomes `withInstanceDefaultAuthentication(...)`. - The README prerequisite now says "QQQ application (v4.0.0+)". It said v0.35.0+. - `76755fe` **build: add opt-in qqq-snapshot profile**. It imports `qqq-bom-pom:${qqq.snapshot.version}` (default `4.1.0-SNAPSHOT`) ahead of the parent's BOM and adds the Central snapshots repository. The parent has no qqq-version property to override. Default builds take the qqq version only from the parent. This profile matches the one in the B3 re-pin (qbit-customizable-table-views#7). - `57ffb1c` **ci: keep package installs unattended on CircleCI**. Adds the `configure-package-install` command from qbit-quick-search `develop` and runs it as a `pre-steps` entry on all 5 jobs. It stops `install_java` from hanging on the needrestart "Daemons using outdated libraries" prompt (QRun-IO/qqq-orb#11). The command block is identical to quick-search's, which I checked with `yq`. The indentation follows this file's 2-space style. - `fe32798` **docs: refresh repo notes for the parent 2.0.0 re-pin**. Replaces the stale "develop and main have diverged / two compile breaks" cautions in `CLAUDE.md` with how the qqq version is set and checked now. The other plan items needed no change. The child pom had no `qqq-bom-pom` import shadowing the parent, and it pins no explicit qqq module `<version>`s (`qqq-backend-core` and `qqq-backend-module-filesystem` are versionless). There are no Docker-backed tests: all 12 tests use the in-memory and local filesystem backends, so the "fail, not skip, when `CI=true`" rule has nothing to change. Under `CI=true`, 0 tests are skipped. ### Merge conflict resolutions | File | develop | main | Resolution | |---|---|---|---| | `LICENSE` (add/add) | AGPL-3.0 with a QRun-IO preamble (`4670ba6`, 2025-12-25) | Apache-2.0 (`7fd2506`, 2026-01-05) | **Took main.** It is the owner's later decision on the released branch, and it matches main's `NOTICE`, which merged in cleanly. Keeping develop's AGPL text beside an Apache `NOTICE` would contradict itself. I wrote no new license text. The pom `<licenses>`, file headers and checkstyle header template are untouched and still say AGPL; aligning them is the separate owner decision the plan calls for. Please confirm this choice in review. | | `.circleci/config.yml` | `kingsrook/qqq-orb@0.3.8` | `kingsrook/qqq-orb@0.5.0` (Java 21 PR #2) | **Took main.** 0.5.0 is the orb that installs Java 21. I did not bump it further (quick-search is on 0.5.2). | | `pom.xml` parent | `qbit-build-parent` 1.4.0 (Java 17) | 1.5.1 (Java 21) | **Took main.** It is the newer Java 21 parent. `3bb034c` then moves it to 2.0.0. | | `pom.xml` `<properties>` | Moved the block to the top, `revision` 0.2.1 | Left it below `<scm>`, `revision` 0.3.0 | **Kept develop's layout with main's value, 0.3.0**, so there is one `<properties>` block. 0.3.0 is already released, and develop's 0.2.1 is older. On `develop`, the orb's `calculate_version.sh` turns a stable version into the next minor snapshot, so develop publishes 0.4.0-SNAPSHOT. It does not re-publish 0.3.0. I did not pick the next release number. | Everything else merged cleanly: `NOTICE`, `.github/release.yml` and `README.md` came from main, and `CODEOWNERS` and `CLAUDE.md` came from develop. The merge commit itself built green (row 1 below). ## Related Issues Closes QRun-IO/qqq#770 Part of QRun-IO/qqq-all#1 ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected): consumers now get qqq 4.0.0 transitively - [x] Documentation update - [ ] Refactoring (no functional changes) - [x] Dependencies update ## Testing I ran every command from the repo root with Maven 3.9.16 and OpenJDK 21.0.12.1. There is no `mvnw`, so I used `mvn`. I never ran `mvn install`. | # | Command | Result | |---|---|---| | 1 | `mvn -B clean verify` on the merge result (parent 1.5.1) | BUILD SUCCESS, 10 tests, qqq 0.35.0 | | 2 | `mvn -B clean verify` with the new tests (parent 1.5.1) | BUILD SUCCESS, 12 tests | | 3 | `mvn -B test -Dtest='SyncSFTP*ConfigScheduledJobTransformStepTest'` with the time zone check disabled in both customizers (temporary, reverted) | **2 failures**, both in `testCronExpressionWithoutTimeZoneIsRejected` (`expected: <1> but was: <0>`) | | 4 | Same command with the old-record map replaced by `Optional.empty()` (temporary, reverted) | **2 failures**, both in `testInsertThenUpdateTwice` (`expected: <1 1 1 * * ?> but was: <0 0 0 * * ?>`) | | 5 | Parent set to 2.0.0, then `mvn -B clean verify` | **BUILD FAILURE**: compile error, `getValueFromRecordOrOldRecord` not found in both customizers | | 6 | After the main-source migration, `mvn -B clean verify` | **BUILD FAILURE**: test compile error, `QInstance.setAuthentication` not found in `BaseTest` | | 7 | After the test migration, `mvn -B clean verify` | BUILD SUCCESS, 12 tests, qqq 4.0.0 | | 8 | Before the profile, `mvn -B dependency:list -Pqqq-snapshot -DincludeGroupIds=com.kingsrook.qqq` | `The requested profile "qqq-snapshot" could not be activated`, resolves `4.0.0` (red) | | 9 | After the profile, same command | `qqq-backend-core` and `qqq-backend-module-filesystem` `4.1.0-SNAPSHOT`. Without `-Pqqq-snapshot`: `4.0.0` | | 10 | `mvn -B dependency:list -Pqqq-snapshot -Dqqq.snapshot.version=0.35.0 -DincludeGroupIds=com.kingsrook.qqq` | `0.35.0`, so the version override works | Final gates on HEAD `fe32798`: | Gate | Command | Result | |---|---|---| | qqq 4.0.0 (parent) | `CI=true mvn -B clean verify` | BUILD SUCCESS, 12 tests, 0 failures, 0 errors, 0 skipped | | qqq 4.1.0-SNAPSHOT | `CI=true mvn -B clean verify -Pqqq-snapshot` | BUILD SUCCESS, 12 tests, 0 failures, 0 errors, 0 skipped | | qqq 4.1.0-SNAPSHOT, published artifacts only | `CI=true mvn -B -U clean verify -Pqqq-snapshot -Dmaven.repo.local=/tmp/m2-b8` (empty local repo) | BUILD SUCCESS, 12 tests, 0 failures. Resolved `qqq-backend-core-4.1.0-20260926.013614-9` from `sonatype-snapshots` | | Java 21 | `javap -v` on a compiled class | class file major version 65 | I also ran `CI=true mvn -B verify` and `CI=true mvn -B verify -Pqqq-snapshot` without `clean`: both BUILD SUCCESS, 12 tests. I added the isolated-repo gate because the shared `~/.m2` can hold locally installed qqq snapshots; it shows the build passes against the published Sonatype snapshot. CircleCI config: `circleci config validate` and the CircleCI config helper both stop at "Cannot find kingsrook/qqq-orb@0.5.0 in the orb registry". They report the same thing for develop's untouched config (orb 0.3.8), so it is a private-orb lookup limit and not a config error. `yq` parses the file, and all 5 jobs list the `configure-package-install` pre-step. On this PR, CircleCI `qqq-orb/mvn_test_only` (build 32) passed in 1m21s, so the orb resolves and `install_java` no longer hangs on the needrestart prompt. - [x] Unit tests pass (`mvn test`) - [x] Integration tests pass (`mvn verify`) - [ ] Manual testing performed ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas (no new logic; the profile and CI step carry comments) - [x] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings (see the parent warning below) - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes ## Screenshots (if applicable) Not applicable. ## Additional Notes - **Merge with a merge commit** (see the Summary). - **Parent warning, not fixed here:** every build logs `[WARNING] Failed to build parent project for com.kingsrook.qbits:qbit-sftp-data-integration:jar:0.3.0`. It already appeared on parent 1.5.1 in this repo. The published parent pom declares its `<version>` as the literal `${revision}`. The effective model is still correct (qqq 4.0.0, Java 21). The fix belongs in `qbit-bom`. - **JUnit stays on 5.10.0:** the child pom imports `junit-bom:5.10.0`, which wins over the parent's `junit-bom:6.0.1`. Tests run on JUnit 5.10.0. I left it alone because the plan only asks to remove shadowing `qqq-bom-pom` imports. - **Out of scope, left as is:** the pom `<licenses>`, the Java source headers, the pom header comment and `checkstyle/license.txt` (all still AGPL; a separate owner decision), the `Kingsrook` URLs in `<url>` and `<scm>`, the orb version beyond main's 0.5.0, and the open Dependabot alert for the test-scoped `assertj-core` 3.27.3 (the existing Dependabot branch covers it). - Not merged. This needs review first.
KofTwentyTwo commented 2026-09-26 01:58:51 +00:00 (Migrated from github.com)
Copy link
Copy source

Review, round 1 (task B8)

Verdict: approve. No blocking issues. James still needs to confirm the license files from the back-merge (non-blocking item 1).

Routing: Deep tier (1,420 changed lines across 12 files, most of them LICENSE and README.md). Risk signals: build/release (.circleci/config.yml, .github/release.yml) and dependencies (pom.xml). I reviewed code, tests, comments and error handling, and did a security pass on the diff.

B8 requirements

Step Status Evidence
1. Back-merge main into develop Met 133e22b is a real merge (parents 3611d54 develop, 7fd2506 main). HEAD..origin/main is empty. Against develop, the merge changes only .circleci/config.yml, .github/release.yml, LICENSE, NOTICE, README.md and two pom lines. It changes no source or test file, so develop's features are all kept.
2. Parent 2.0.0, no shadowing BOM, Java 21, headers untouched Met Parent 2.0.0, and the child pom has no qqq-bom-pom import. The effective POM has java.version 21. No header lines change under src/ and checkstyle/.
3. Green on 4.0.0, plus the qqq-snapshot profile Met The profile matches the plan: qqq-bom-pom:${qqq.snapshot.version}, default 4.1.0-SNAPSHOT, Central snapshots repo, releases disabled. dependency:list resolves 4.0.0 without the profile and 4.1.0-SNAPSHOT with it.
4. Docker ITs fail, not skip, when CI=true Nothing to change There are no Testcontainers, Docker, @Disabled or assume* uses under src/test. 0 tests are skipped.
5. No explicit qqq module <version>s Met qqq-backend-core and qqq-backend-module-filesystem have no <version>.

What I ran (on fe32798)

  • CI=true mvn -B clean verify: BUILD SUCCESS, 12 tests, 0 failures, 0 skipped.
  • CI=true mvn -B clean verify -Pqqq-snapshot: BUILD SUCCESS, 12 tests, 0 failures, 0 skipped.
  • Mutation checks in a scratch copy, reverted afterwards:
    • Removed the old-record fallback (record.getValue("cronTimeZoneId") only). testInsertThenUpdateTwice fails in both test classes.
    • Defaulted a missing time zone to "UTC". The new testCronExpressionWithoutTimeZoneIsRejected fails in both classes.
    • Both lookup branches are covered, so the tests exercise the migrated behavior.
  • The migration keeps the same lookup. In qqq v0.35.0, getValueFromRecordOrOldRecord is @Deprecated in favor of getValueFromRecordElseFromOldRecord wrapped in ValueUtils.getValueAsXyz. In v4.0.0, that method has the same body minus the unchecked cast. No uses of getValueFromRecordOrOldRecord or setAuthentication( remain.
  • Checkstyle and coverage: neither qbit-build-parent 2.0.0 nor this pom configures checkstyle or JaCoCo, so no gate can fail. develop has none either.
  • CircleCI qqq-orb/mvn_test_only passes on the head commit. All 5 jobs have the configure-package-install pre-step. The needrestart line is single-quoted, so the shell leaves $nrconf alone.
  • Secrets: none in the diff. The only matches are the ${SFTP_PASSWORD} and ${KEY_PASSPHRASE} placeholders in main's README. The CI config names the existing context and holds no credentials.
  • Merge commits are enabled on this repo, so the "merge commit, not squash" instruction can be followed.

Non-blocking

  1. James needs to confirm the license files. Taking main's side changes develop's LICENSE from AGPL-3.0 to Apache-2.0 and adds NOTICE. Read literally, the plan says not to change LICENSE files. But 7fd2506 on main is the owner's own "migrate license from AGPL-3.0 to Apache-2.0" commit. Keeping develop's side would put AGPL back on main at the next release. This is the same call as B2 and B4, so I'm not blocking on it. Also note that develop now carries three different license statements:

    • LICENSE: Apache-2.0
    • pom <licenses>, source headers and checkstyle/license.txt: AGPL
    • README.md: "Proprietary - QRun.IO", from main

    The new CLAUDE.md note lists the first two but not the README line.

  2. The README from main documents APIs this QBit doesn't have. README.md merged cleanly from main. It shows these, and none of them exist in src/:

    • new SftpDataIntegrationQBit().configure(qInstance)
    • QSftpConnectionMetaData, QSftpImportMetaData, QSftpExportMetaData
    • PostProcessAction, withPrivateKeyPath
    • tables sftp_file_log and sftp_error_log

    Develop's old README had the real setup: SFTPDataIntegrationQBitConfig and SFTPDataIntegrationQBitProducer, the source/staging/destination table configs, withSchedulerName, and sftpPrivateKeyEnvVarName. After this merge, develop has no accurate usage docs. No conflict flagged it, and this PR edits the README's prerequisite line, so it is easy to miss. I suggest a follow-up that restores develop's setup and architecture sections on both branches. Fixing it here is fine too.

  3. CLAUDE.md wording. "check the next line with mvn -B verify -Pqqq-snapshot" can read as "the next line of this file". Suggest: "check against the next qqq line (4.1.0-SNAPSHOT) with mvn -B verify -Pqqq-snapshot".

  4. JUnit stays on 5.10.0. The child's junit-bom:5.10.0 import shadows the parent's 6.0.1, as the PR says. Leaving it is within scope. It is worth a follow-up so the qbits share one JUnit line.

  5. revision is 0.3.0 on develop, the same as the released version. The PR explains that the orb publishes it as 0.4.0-SNAPSHOT on develop, so this is fine. Pick the next release number when cutting the release branch.

  6. Existing warning: Failed to build parent project ... 0.3.0 comes from the published parent's literal ${revision} version. It belongs in qbit-bom, as the PR notes.

Not merged. Merging is James's call.

## Review, round 1 (task B8) **Verdict: approve. No blocking issues.** James still needs to confirm the license files from the back-merge (non-blocking item 1). Routing: Deep tier (1,420 changed lines across 12 files, most of them `LICENSE` and `README.md`). Risk signals: build/release (`.circleci/config.yml`, `.github/release.yml`) and dependencies (`pom.xml`). I reviewed code, tests, comments and error handling, and did a security pass on the diff. ### B8 requirements | Step | Status | Evidence | |---|---|---| | 1. Back-merge `main` into `develop` | Met | `133e22b` is a real merge (parents `3611d54` develop, `7fd2506` main). `HEAD..origin/main` is empty. Against `develop`, the merge changes only `.circleci/config.yml`, `.github/release.yml`, `LICENSE`, `NOTICE`, `README.md` and two pom lines. It changes no source or test file, so develop's features are all kept. | | 2. Parent 2.0.0, no shadowing BOM, Java 21, headers untouched | Met | Parent `2.0.0`, and the child pom has no `qqq-bom-pom` import. The effective POM has `java.version` 21. No header lines change under `src/` and `checkstyle/`. | | 3. Green on 4.0.0, plus the `qqq-snapshot` profile | Met | The profile matches the plan: `qqq-bom-pom:${qqq.snapshot.version}`, default `4.1.0-SNAPSHOT`, Central snapshots repo, releases disabled. `dependency:list` resolves `4.0.0` without the profile and `4.1.0-SNAPSHOT` with it. | | 4. Docker ITs fail, not skip, when `CI=true` | Nothing to change | There are no Testcontainers, Docker, `@Disabled` or `assume*` uses under `src/test`. 0 tests are skipped. | | 5. No explicit qqq module `<version>`s | Met | `qqq-backend-core` and `qqq-backend-module-filesystem` have no `<version>`. | ### What I ran (on `fe32798`) - `CI=true mvn -B clean verify`: BUILD SUCCESS, 12 tests, 0 failures, 0 skipped. - `CI=true mvn -B clean verify -Pqqq-snapshot`: BUILD SUCCESS, 12 tests, 0 failures, 0 skipped. - Mutation checks in a scratch copy, reverted afterwards: - **Removed the old-record fallback** (`record.getValue("cronTimeZoneId")` only). `testInsertThenUpdateTwice` fails in both test classes. - **Defaulted a missing time zone to `"UTC"`.** The new `testCronExpressionWithoutTimeZoneIsRejected` fails in both classes. - Both lookup branches are covered, so the tests exercise the migrated behavior. - The migration keeps the same lookup. In qqq `v0.35.0`, `getValueFromRecordOrOldRecord` is `@Deprecated` in favor of `getValueFromRecordElseFromOldRecord` wrapped in `ValueUtils.getValueAsXyz`. In `v4.0.0`, that method has the same body minus the unchecked cast. No uses of `getValueFromRecordOrOldRecord` or `setAuthentication(` remain. - Checkstyle and coverage: neither `qbit-build-parent` 2.0.0 nor this pom configures checkstyle or JaCoCo, so no gate can fail. `develop` has none either. - CircleCI `qqq-orb/mvn_test_only` passes on the head commit. All 5 jobs have the `configure-package-install` pre-step. The needrestart line is single-quoted, so the shell leaves `$nrconf` alone. - Secrets: none in the diff. The only matches are the `${SFTP_PASSWORD}` and `${KEY_PASSPHRASE}` placeholders in main's README. The CI config names the existing context and holds no credentials. - Merge commits are enabled on this repo, so the "merge commit, not squash" instruction can be followed. ### Non-blocking 1. **James needs to confirm the license files.** Taking main's side changes develop's `LICENSE` from AGPL-3.0 to Apache-2.0 and adds `NOTICE`. Read literally, the plan says not to change LICENSE files. But `7fd2506` on main is the owner's own "migrate license from AGPL-3.0 to Apache-2.0" commit. Keeping develop's side would put AGPL back on `main` at the next release. This is the same call as B2 and B4, so I'm not blocking on it. Also note that develop now carries three different license statements: - `LICENSE`: Apache-2.0 - pom `<licenses>`, source headers and `checkstyle/license.txt`: AGPL - `README.md`: "Proprietary - QRun.IO", from main The new `CLAUDE.md` note lists the first two but not the README line. 2. **The README from main documents APIs this QBit doesn't have.** `README.md` merged cleanly from main. It shows these, and none of them exist in `src/`: - `new SftpDataIntegrationQBit().configure(qInstance)` - `QSftpConnectionMetaData`, `QSftpImportMetaData`, `QSftpExportMetaData` - `PostProcessAction`, `withPrivateKeyPath` - tables `sftp_file_log` and `sftp_error_log` Develop's old README had the real setup: `SFTPDataIntegrationQBitConfig` and `SFTPDataIntegrationQBitProducer`, the source/staging/destination table configs, `withSchedulerName`, and `sftpPrivateKeyEnvVarName`. After this merge, develop has no accurate usage docs. No conflict flagged it, and this PR edits the README's prerequisite line, so it is easy to miss. I suggest a follow-up that restores develop's setup and architecture sections on both branches. Fixing it here is fine too. 3. **`CLAUDE.md` wording.** "check the next line with `mvn -B verify -Pqqq-snapshot`" can read as "the next line of this file". Suggest: "check against the next qqq line (`4.1.0-SNAPSHOT`) with `mvn -B verify -Pqqq-snapshot`". 4. **JUnit stays on 5.10.0.** The child's `junit-bom:5.10.0` import shadows the parent's 6.0.1, as the PR says. Leaving it is within scope. It is worth a follow-up so the qbits share one JUnit line. 5. **`revision` is 0.3.0 on develop**, the same as the released version. The PR explains that the orb publishes it as `0.4.0-SNAPSHOT` on develop, so this is fine. Pick the next release number when cutting the release branch. 6. **Existing warning:** `Failed to build parent project ... 0.3.0` comes from the published parent's literal `${revision}` version. It belongs in `qbit-bom`, as the PR notes. Not merged. Merging is James's call.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 dependencies
Pull requests that update a dependency file
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 java
Pull requests that update java code
 question
Further information is requested
 release:rc
Merging this develop PR cuts the next release candidate
 wontfix
This will not be worked on
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
forge-recovery (Forge Recovery) james.maes (James Maes)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: QRun-IO/qbit-sftp-data-integration#4