SOL-59692: Provide Ability to NACK a Specific AD Message and Force Re-delivery #66
Conversation
…lement outcome support
oodigie
requested review from
cjwmorgan-sol,
RagnarPaulson,
TrentDaniel and
sdewilde
|
Please mark whether you used Copilot to assist coding in this PR
|
Still more reshuffling than substantial tests, but I'm getting there.
More happy cases, plus a small bug they uncoverred fixed.
fixed the failing unit test
Ebp 79 nack tests
| @@ -105,8 +108,9 @@ type Metrics interface { | |||
|
|
|||
| // Implementation | |||
| type ccsmpBackedMetrics struct { | |||
| session *ccsmp.SolClientSession | |||
| metrics []uint64 | |||
| session *ccsmp.SolClientSession | |||
There was a problem hiding this comment.
Which indentation is our coding standard, before or after?
There was a problem hiding this comment.
Go has its own standard for formatting for running go fmt on the project.
Not sure indentation is included in that for all cases.
If go fmt is happy will this the indentation standard for solace by default is 4 spaces. Unless there is overriding factor like go fmt.
There was a problem hiding this comment.
@gszol, I just run the usual go fmt on the code before/while committing the code changes.
|
I think the question here is on the alignment within the definitions because it looks like we have deleted one line and change the other line to include a tab between the var name and the object type.
Seeing as Go appears to put the var-name first and type second, it’s not that big a deal but would be good to be consistent.
In C where the var-name is second, it’s nice to align the names which means using a tab to account for different string length on type. Eg in C I prefer
int x;
char *y;
Char* b;
The third is the easiest way to align pointer names but slightly hides the `*`
Anyway this not ‘C’ so is there value in aligning the types?
Mostly I just suggest be consistent, especially within an object definition.
Ragnar
From: Chris Morgan ***@***.***>
Sent: November 5, 2024 3:27 PM
To: SolaceDev/pubsubplus-go-client ***@***.***>
Cc: Ragnar Paulson (he/him) ***@***.***>; Review requested ***@***.***>
Subject: Re: [SolaceDev/pubsubplus-go-client] SOL-59692: Provide Ability to NACK a Specific AD Message and Force Re-delivery (PR #66)
Warning - this email originated outside of Solace. Please think before you click or open any links or attachments. If this email is suspicious, report it to ***@***.******@***.***>
@cjwmorgan-sol commented on this pull request.
________________________________
In internal/impl/core/metrics.go<#66 (comment)>:
@@ -105,8 +108,9 @@ type Metrics interface {
// Implementation
type ccsmpBackedMetrics struct {
- session *ccsmp.SolClientSession
- metrics []uint64
+ session *ccsmp.SolClientSession
Go has its own standard for formatting for running go fmt on the project.
Not sure indentation is included in that for all cases.
If go fmt is happy will this the indentation standard for solace by default is 4 spaces. Unless there is overriding factor like go fmt.
—
Reply to this email directly, view it on GitHub<#66 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AISWP2OKGJVJYPXSJIUTWITZ7ESXZAVCNFSM6AAAAABPVDPASKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMJWGYZDINJXGA>.
You are receiving this because your review was requested.Message ID: ***@***.******@***.***>>
…________________________________
Confidentiality notice
This e-mail message and any attachment hereto contain confidential information which may be privileged and which is intended for the exclusive use of its addressee(s). If you receive this message in error, please inform sender immediately and destroy any copy thereof. Furthermore, any disclosure, distribution or copying of this message and/or any attachment hereto without the consent of the sender is strictly prohibited. Thank you.
|
|
Changes look good to me. |
There was a problem hiding this comment.
Merge jira has been raised https://sol-jira.atlassian.net/browse/EBP-368. Ready to go!
Changes: