Improve/websock and cr#300
Conversation
…o improve/websock-and-cr
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @abdheshnayak - I've reviewed your changes and they look great!
General suggestions:
- Consider adding comments to clarify the business or technical reasons behind allowing
RecordVersionto be nil and its impact on theLogSubsIdfunction's output format. - Ensure that the new format for the log subscription ID, especially the inclusion of a trailing
>, is consistent with the expectations of all systems consuming these IDs. - Review the handling of
RecordVersionbeing nil across the application to ensure that this change integrates smoothly with existing functionalities.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| if md.RecordVersion == nil { | ||
| return fmt.Sprintf("%s.%s.%s.%s.>", logStreamName, md.Account, md.Cluster, md.TrackingId) | ||
| } |
There was a problem hiding this comment.
suggestion (code_clarification): Consider adding a comment explaining why a nil RecordVersion leads to a different format in LogSubsId. This could help maintainers understand the business logic or technical reasons behind this decision.
| func LogSubsId(md MsgData, logStreamName string) string { | ||
|
|
||
| if md.RecordVersion == nil { | ||
| return fmt.Sprintf("%s.%s.%s.%s.>", logStreamName, md.Account, md.Cluster, md.TrackingId) |
There was a problem hiding this comment.
question (typo): The format string ends with a >. If this is intentional, ensure that it aligns with the expected format for all consumers of this ID. If not, it might be a typo that could lead to inconsistencies in ID formatting.
1 participant
No description provided.