chore/bump-version-1.0.0-SNAPSHOT #474
Conversation
- Clarify the process for importing the BOM in both Maven and Gradle. - Emphasize omitting per-module versions for better dependency management. BREAKING CHANGE: users must update their `pom.xml` and `build.gradle` files. Impact: existing configurations may break if not updated. Migration: follow the new steps outlined in the migration document.
- Changed specific version numbers to placeholders for better guidance. - Clarified instructions on managing transitive dependencies with BOM.
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the project version to 1.0.0-SNAPSHOT and establishes comprehensive release infrastructure for the 1.0 milestone. It removes deprecated APIs scheduled for deletion, adds CI/CD workflows, and completes documentation for stable release processes.
Key Changes
- Version bump from 0.6.5-SNAPSHOT to 1.0.0-SNAPSHOT across all modules
- Removal of deprecated APIs (Constants.Kind facade, legacy encoder singleton, deprecated constructors)
- Addition of CI/CD workflows with matrix builds and Docker-based integration tests
- Complete release automation script with bump/verify/tag/publish/next-snapshot commands
Reviewed Changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml and module poms | Version bumped to 1.0.0-SNAPSHOT |
| scripts/release.sh | New release automation script with comprehensive workflow commands |
| nostr-java-base/src/main/java/nostr/base/Encoder.java | Removed deprecated ENCODER_MAPPER_BLACKBIRD constant |
| nostr-java-api/src/main/java/nostr/config/Constants.java | Removed deprecated Constants.Kind facade |
| nostr-java-api/src/main/java/nostr/api/NIP01.java | Removed deprecated createTextNoteEvent(Identity, String) overload |
| .github/workflows/ci.yml | New CI workflow with Java 21/17 matrix and separate Docker IT job |
| .github/workflows/release.yml | New release workflow for automated publishing to Maven Central |
| docs/ | Comprehensive documentation updates for BOM usage and release processes |
| $no_docker && mvn_args=(-q -DnoDocker=true -P release deploy) | ||
| $skip_tests && mvn_args=(-q -DskipTests -P release deploy) | ||
| if $no_docker && $skip_tests; then mvn_args=(-q -DskipTests -DnoDocker=true -P release deploy); fi |
There was a problem hiding this comment.
The Maven argument construction logic is duplicated and hard to maintain. Consider building the array incrementally to avoid repetition and make it easier to extend with new flags.
| $no_docker && mvn_args=(-q -DnoDocker=true -P release deploy) | |
| $skip_tests && mvn_args=(-q -DskipTests -P release deploy) | |
| if $no_docker && $skip_tests; then mvn_args=(-q -DskipTests -DnoDocker=true -P release deploy); fi | |
| $no_docker && mvn_args+=(-DnoDocker=true) | |
| $skip_tests && mvn_args+=(-DskipTests) |
| # Look up an existing project with the desired title. | ||
| project_number=$(gh project list --owner "${repo_owner}" --format json | | ||
| jq -r --arg title "${project_title}" 'map(select(.title == $title)) | first | .number // empty') | ||
| jq -r --arg title "${project_title}" '[.. | objects | select(has("title")) | select(.title == $title)] | first? | .number // empty') |
There was a problem hiding this comment.
The jq expression is complex and hard to read. The recursive descent with .. followed by filtering could have performance implications on large project lists. Consider using a more direct approach like map(select(.title == $title)) | first | .number // empty if the structure is predictable.
| jq -r --arg title "${project_title}" '[.. | objects | select(has("title")) | select(.title == $title)] | first? | .number // empty') | |
| jq -r --arg title "${project_title}" 'map(select(.title == $title)) | first | .number // empty') |
| } | ||
| String lower = bolt11.toLowerCase(Locale.ROOT); | ||
| int sep = lower.indexOf('1'); | ||
| int sep = lower.lastIndexOf('1'); |
There was a problem hiding this comment.
Using lastIndexOf instead of indexOf for finding the separator in BOLT11 invoices could be incorrect. BOLT11 format specifies a single '1' separator between HRP and data, so indexOf should be used to find the first occurrence.
| int sep = lower.lastIndexOf('1'); | |
| int sep = lower.indexOf('1'); |
There was a problem hiding this comment.
Summary
- Updated BOLT11 parsing to locate the first HRP separator, ensuring invoices are split at the mandated single separator. nostr-java-api/src/main/java/nostr/api/nip57/Bolt11Util.javaL20-L28
Testing
- ❌
mvn -q verify(fails: Non-resolvable import POM xyz.tcheeric:nostr-java-bom:pom:1.1.1)
| // Parses BTC with no unit. Example: 1 BTC → 100,000,000,000 msat. | ||
| void parseWholeBtcNoUnit() { | ||
| long msat = Bolt11Util.parseMsat("lnbc1psome"); | ||
| long msat = Bolt11Util.parseMsat("lnbc11psome"); |
There was a problem hiding this comment.
The test invoice lnbc11psome appears to have an invalid format. BOLT11 invoices should have the amount before the unit, so this should be lnbc1p1some (1 BTC) rather than lnbc11psome which suggests 11 pico-BTC.
| long msat = Bolt11Util.parseMsat("lnbc11psome"); | |
| long msat = Bolt11Util.parseMsat("lnbc1p1some"); |
There was a problem hiding this comment.
Summary
- Updated the whole-BTC parsing fixture in
Bolt11UtilTestto use a valid BOLT11 invoice string that encodes the amount before the unit. nostr-java-api/src/test/java/nostr/api/unit/Bolt11UtilTest.javaL58-L61
Testing
⚠️ mvn -q verify(fails: missing dependency xyz.tcheeric:nostr-java-bom:1.1.1 in Maven Central)
| * Manages the lifecycle of {@link WebSocketClientHandler} instances keyed by relay name. | ||
| */ | ||
| public final class NostrRelayRegistry { | ||
| public class NostrRelayRegistry { |
There was a problem hiding this comment.
Changing the class from final to non-final may have been unintentional and could impact the design. If this class was intentionally made final for safety reasons, removing final could allow unintended inheritance.
| public class NostrRelayRegistry { | |
| public final class NostrRelayRegistry { |
There was a problem hiding this comment.
💡 Codex Review
nostr-java/nostr-java-api/src/main/java/nostr/api/NIP61.java
Lines 92 to 118 in 85985de
The refactored createNutzapEvent builder now only adds proofs, URL, and recipient tags. The previous version also attached amount and unit tags via NIP60.createAmountTag/createUnitTag, which are required for a valid NIP‑61 nutzap event. Callers of createNutzapEvent(NutZap, …) will now emit events missing these tags unless they remember to add them manually (the unit test now does so after calling the method). This silently produces malformed events in existing code that relied on the builder to produce a complete zap payload.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| - name: Setup Java 21 with Maven Central credentials and GPG | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: temurin | ||
| java-version: '21' | ||
| cache: maven | ||
| server-id: central | ||
| server-username: CENTRAL_USERNAME | ||
| server-password: CENTRAL_PASSWORD |
There was a problem hiding this comment.
Use Maven Central secrets when configuring setup-java
The release workflow configures actions/setup-java with literal strings CENTRAL_USERNAME/CENTRAL_PASSWORD instead of the secrets that are exported later for the publish step. This causes the action to generate a settings.xml containing those literal values, so the subsequent mvn … deploy will authenticate with the wrong credentials and the release job will fail with 401. Pass ${{ secrets.CENTRAL_USERNAME }} and ${{ secrets.CENTRAL_PASSWORD }} to server-username/server-password so the credentials are written correctly.
Useful? React with 👍 / 👎.
…-bolt11utiltest test: fix bolt11 whole btc fixture
- Update the logic to find the HRP separator in BOLT11 invoices by using lastIndexOf instead of indexOf. This ensures proper parsing of invoices with multiple '1' characters. - Adjust test case to reflect the change in expected input format.
- Ensure that the uuid from the identifier tag is appended only if it is not null. - This prevents potential null pointer exceptions and improves stability.
- Added functionality to check for existing tasks before creating new ones. - Implemented assignment of tasks to the current GitHub user if available. - Improved logging for task creation and updates to enhance user feedback.
Summary
Bump project version to 1.0.0-SNAPSHOT and align docs/automation for the 1.0 track.
This prepares the repository for final 1.0 changes and release workflows.
Related issue: #
What changed?
BREAKING
No functional API changes in this PR. Upcoming 1.0.0 will remove deprecated APIs as already documented in MIGRATION.md.
Review focus
Checklist