Add message retention related data types#2
Conversation
rnons
commented
May 29, 2026
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR extends the Slack API client with message retention support across two layers: it adds a ConversationRetention type and API methods to fetch retention policies for individual conversations, and it augments the TeamPrefs struct with retention configuration fields for public channels, private channels, and direct messages. ChangesMessage Retention Support
🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| GroupRetentionType int `json:"group_retention_type,omitempty"` | ||
| GroupRetentionDuration int `json:"group_retention_duration,omitempty"` | ||
| DMRetentionType int `json:"dm_retention_type,omitempty"` | ||
| DMRetentionDuration int `json:"dm_retention_duration,omitempty"` |
There was a problem hiding this comment.
Type inconsistency with surrounding TeamPrefs fields: the existing prefs in this struct use pointer types (*int, *bool) so callers can distinguish an absent field from a zero value. The new retention fields use plain int with omitempty, which means a JSON payload that omits e.g. retention_type is indistinguishable from one that sends 0. Per your own doc comment, value 0 is meaningful (other values mean no automatic deletion), so an unset field and an explicit unset look the same to callers. Consider matching the existing style with *int for symmetry and to preserve the absent-vs-zero signal.
| // Message retention defaults. The *_type fields use 1 = retain all messages, | ||
| // 2 = delete after the matching *_duration (in days); other values mean no | ||
| // automatic deletion. Public channels use RetentionType/RetentionDuration, | ||
| // private channels use the Group* fields and DMs/MPIMs use the DM* fields. |
There was a problem hiding this comment.
Doc inconsistency with ConversationRetention.Type: the comment over in conversation.go enumerates 0/1/2/3 with concrete meanings (0=inherit workspace default, 3=custom timeline), but here "other values" is left vague. Since both fields are decoding the same Slack retention enum it would be clearer to use the same enumeration here so callers know about the 3 and 0 cases.