Skip to content

refactor(foreign/go): replace the previous implementation of IggyMessage to option pattern#1904

Merged
spetz merged 7 commits into
apache:masterfrom
chengxilo:golang-sdk-iggymessage-refactor
Jun 24, 2025
Merged

refactor(foreign/go): replace the previous implementation of IggyMessage to option pattern#1904
spetz merged 7 commits into
apache:masterfrom
chengxilo:golang-sdk-iggymessage-refactor

Conversation

@chengxilo
Copy link
Copy Markdown
Contributor

@chengxilo chengxilo commented Jun 23, 2025 •
edited
Loading

This pr :

  1. changed the previous implementation of NewIggyyMessage to option pattern
  2. adds some parameter validate on creating the message.
  3. editted those test file which was affected.

@chengxilo chengxilo marked this pull request as draft June 23, 2025 14:22
@chengxilo chengxilo marked this pull request as ready for review June 23, 2025 14:35
@chengxilo chengxilo marked this pull request as draft June 23, 2025 14:37
@chengxilo chengxilo marked this pull request as ready for review June 23, 2025 14:47
@chengxilo
Copy link
Copy Markdown
Contributor Author

chengxilo commented Jun 23, 2025

On sorry I just realized that there is a directory for error. Will convert to draft and fix the error handling.

@chengxilo chengxilo marked this pull request as draft June 23, 2025 15:11
@chengxilo chengxilo marked this pull request as ready for review June 23, 2025 15:23
@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented Jun 24, 2025

@haze518 mind taking a look?

@haze518
Copy link
Copy Markdown
Member

haze518 commented Jun 24, 2025

@haze518 mind taking a look?

sure thing, i'll take a look today

haze518
haze518 reviewed Jun 24, 2025
View reviewed changes
Comment thread foreign/go/benchmarks/send_messages_benchmark_test.go
Comment thread foreign/go/contracts/message_header.go Outdated
Comment thread foreign/go/e2e/tcp_test/messages_steps.go Outdated
Comment thread foreign/go/contracts/messages.go Outdated
@chengxilo
Copy link
Copy Markdown
Contributor Author

chengxilo commented Jun 24, 2025

@haze518 Thanks for your adivce, I will update the code.

chengxilo added 2 commits June 24, 2025 10:43
@chengxilo
refactor: change [16]byte in message header to custom type MessageID.
2cfe9fe 
doc: add comment for MaxPayloadSize and MaxUserHeaderSize.
@chengxilo
Merge remote-tracking branch 'origin/golang-sdk-iggymessage-refactor'…
ef11b0d 
… into golang-sdk-iggymessage-refactor
@chengxilo chengxilo requested a review from haze518 June 24, 2025 14:53
@chengxilo
Copy link
Copy Markdown
Contributor Author

chengxilo commented Jun 24, 2025 •
edited
Loading

I finished changing the data type with MessageID and I also added the comment according to rust source code. I think it is ready to be merged now. (of course, after review).

@haze518
Copy link
Copy Markdown
Member

haze518 commented Jun 24, 2025

I finished changing the data type with MessageID and I also added the comment according to rust source code. I think it is ready to be merged now. (of course, after review).

LGTM!

@spetz spetz merged commit 1c9f271 into apache:master Jun 24, 2025
15 checks passed
hageshiame pushed a commit to hageshiame/iggy that referenced this pull request Nov 7, 2025
@chengxilo
refactor(foreign/go): replace the previous implementation of IggyMess…
e1655bf 
…age to option pattern (apache#1904)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spetz spetz spetz approved these changes

@haze518 haze518 haze518 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chengxilo @hubcio @haze518 @spetz