Skip to content

Conversation

@tcheeric
Copy link
Owner

Summary

Related issue: #____

Restored the ability for NIP01EventBuilder to accept per-call senders so the builder can satisfy callers that need to override the default identity, addressing the review feedback.

What changed?

  • Reintroduced sender-aware overloads for all event builder helpers while keeping the default-sender convenience methods.
  • Added a focused unit test suite for NIP01EventBuilder covering override and fallback behavior.

Testing

$ mvn -q verify
[ERROR] [ERROR] Some problems were encountered while processing the POMs:
[ERROR] Non-resolvable import POM: The following artifacts could not be resolved: xyz.tcheeric:nostr-java-bom:pom:1.1.8 (absent): Could not find artifact xyz.tcheeric:nostr-java-bom:pom:1.1.8 in central (https://repo.maven.apache.org/maven2) @ line 99, column 25
 @
[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]
[ERROR]   The project xyz.tcheeric:nostr-java:1.0.0 (/workspace/nostr-java/pom.xml) has 1 error
[ERROR]     Non-resolvable import POM: The following artifacts could not be resolved: xyz.tcheeric:nostr-java-bom:pom:1.1.8 (absent): Could not find artifact xyz.tcheeric:nostr-java-bom:pom:1.1.8 in central (https://repo.maven.apache.org/maven2) @ line 99, column 25 -> [Help 2]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException
[ERROR] [Help 2] http://cwiki.apache.org/confluence/display/MAVEN/UnresolvableModelException

BREAKING

None.

Review focus

Please double-check that the overload placement and generics preserve backward compatibility and align with desired API surface.

Checklist

  • Scope ≤ 300 lines (or split/stack)
  • Title is verb + object (e.g., “Refactor auth middleware to async”)
  • Description links the issue and answers “why now?”
  • BREAKING flagged if needed
  • Tests/docs updated (if relevant)

Network Access

No blocked domains encountered during this work.


https://chatgpt.com/codex/tasks/task_b_68ed9352511883319e4b975b168abe18

@tcheeric tcheeric merged commit b0b8d60 into chore/bump-version-1.0.2-SNAPSHOT Oct 14, 2025
@tcheeric tcheeric deleted the codex/address-code-comment-in-nip01eventbuilder branch October 14, 2025 00:13
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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

Comment on lines 73 to +75
public GenericEvent buildReplaceableEvent(Integer kind, String content) {
return new GenericEventFactory(resolveSender(null), kind, content).create();
return buildReplaceableEvent(null, kind, content);
}

Choose a reason for hiding this comment

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

P0 Badge Disambiguate overloads when delegating

The new helper buildReplaceableEvent(Integer kind, String content) delegates to buildReplaceableEvent(null, kind, content) while two overloads with the same arity now exist (Identity, Integer, String and List<BaseTag>, Integer, String). Because null matches both first-parameter types, this call will not compile (“reference to buildReplaceableEvent is ambiguous”). The same ambiguity exists for buildEphemeralEvent(Integer, String) and buildAddressableEvent(Integer, String) which delegate in the same way. These delegations should cast the null to the intended parameter type or call the implementation directly to keep the API compiling.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants