Skip to content

Multi-team & Slack App support#130

Open
JackCuthbert wants to merge 2 commits intomasterfrom
slack-app
Open

Multi-team & Slack App support#130
JackCuthbert wants to merge 2 commits intomasterfrom
slack-app

Conversation

@JackCuthbert
Copy link
Copy Markdown
Owner

@JackCuthbert JackCuthbert commented Jan 4, 2021

Rewrites the core application to better support updating statuses across multiple teams and be installed as a Slack App.

Todo

  • Update documentation to explain Slack App installation
  • Update & test Docker image
  • Tests?

@JackCuthbert JackCuthbert added the enhancement New feature or request label Jan 4, 2021
Comment thread src/index.ts Outdated
}

if (!config.updateWeekends && isWeekend(currentTime)) {
if (isWeekend(currentTime) && config.app.update_weekends) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad logic, should be:

if (isWeekend(currentTime) && !config.app.update_weekends) { }

Comment thread src/index.ts Outdated
await writeTrackJSON(track)
// Compute new status and duration
// ---
const trackDuration = track?.duration ?? 60 * (10 * 1000) // 10 minutes
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to:

const trackDuration = track?.duration ?? 600000 // 10 minutes

Comment thread src/index.ts Outdated
Comment on lines +100 to +107
for (const client of clients) {
const [presence, profile] = await Promise.all([
client.getPresence(),
client.getProfile()
])

if (presence !== 'active') return
if (!profile.status_text.includes(config.app.separator)) return
Copy link
Copy Markdown
Owner Author

@JackCuthbert JackCuthbert Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary to batch these requests, we can avoid the profile request if the presence is away.

Comment thread src/index.ts
.then(main)
.then(loop)
.catch(handleError)
.catch(console.error)
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process.exit(1) and schedule.stop() here?

@JackCuthbert JackCuthbert force-pushed the slack-app branch 3 times, most recently from e475759 to 4da0956 Compare January 11, 2021 04:34
@JackCuthbert JackCuthbert force-pushed the slack-app branch 7 times, most recently from 41e9795 to b2aabbc Compare January 12, 2021 21:33
BREAKING CHANGE: This requires a completely new configuration format but
everything else should remain the same.

Closes #106
Closes #99
Closes #114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant