Feat: Remove requirement of channel and templates to just manage user groups#25
Feat: Remove requirement of channel and templates to just manage user groups#25halkeye wants to merge 1 commit intotimoreimann:masterfrom
Conversation
… groups * Slight refactor to move validating channel and template data into config object (mostly so the readability of the main sync function gets improved and easier to skip not return) * Update syncer objects to skip updating topic and joining channels when none are defined
timoreimann
left a comment
timoreimann
left a comment
There was a problem hiding this comment.
Thanks for the PR! This is looking pretty good, left just a few comments.
Unfortunately, I don't have a way to e2e-test this either anymore. :( Unless there's a way for you to verify the PR in any staging/prod environment, we may have to go with the amount of testing we have for now.
Re: mocking library: no super strong preference, though I'm not a huge fan of testify's :). We could go with Uber's if that's something you'd like to take on.
| DryRun bool `yaml:"dryRun"` | ||
| } | ||
|
|
||
| func (css *ConfigSlackSync) populateChannel(_ context.Context, allChannels channelList) error { |
There was a problem hiding this comment.
I suppose we could drop the context.Context parameter from the signature here and then again from populateTemplate (unless they are part of some interface that I'm missing)?
There was a problem hiding this comment.
probably, i got in the habit of always including context so you don't have to go back later if things change, but you are right, it doesn't make sense here.
| } else { | ||
| if p.tmplFile != "" { | ||
| b, err := ioutil.ReadFile(p.tmplFile) | ||
| b, err := os.ReadFile(p.tmplFile) |
| } | ||
|
|
||
| if len(slackSync.slackChannelID) == 0 { | ||
| fmt.Printf("no channel for %s slack sync, so skip joining", slackSync.name) |
There was a problem hiding this comment.
I'm inclined to suggest we drop this log line since it could be fairly noisy for channel-less configurations given it is invoked on every sync run. (In contrast, I believe we only log about successfully joining once.)
so looking at https://api.slack.com/developer-program I can sign up for the dev program. I'll give this another poke over the weekend. |
2 participants
I can attempt more tests for createSlackSyncs if you have a prefered mocking library
Also i can't e2e test because my free slack doesn't have usergroups.