Summary

Add an extension for notifying Slack.com

Review Request #6024 — Created June 23, 2014 and submitted June 25, 2014, 5:14 p.m.

Information

Owner

david

Repository

rb-extension-pack

Branch

Bugs

Depends On

Commit

e851f7b...

Reviewers

Groups

reviewboard

People

Description

Slack.com is a popular business chat service which targets software developers.
One of its neat features is that it integrates with a lot of third-party
services, allowing them to send notifications.

This change adds a new extension which integrates Review Board with Slack,
using their "incoming webhooks" integration. This will notify for all of the
usual events, including links to the affected review requests.

Testing Done

Configured this extension in my local devserver and ran through the motions.

Issues

Description	From	Last Updated
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	June 23, 2014, 5:51 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	June 23, 2014, 5:51 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	June 23, 2014, 5:51 p.m.
Shouldn't we copyright Beanbag, Inc.?	chipx86	June 24, 2014, 12:48 a.m.
No blank line here.	chipx86	June 24, 2014, 12:48 a.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	June 23, 2014, 6 p.m.
This is going to break as soon as we do an update that in any way modifies the file. If …	chipx86	June 24, 2014, 1 a.m.
You shouldn't need the default, since RB already defaults to http, and get() will fall back on that.	chipx86	June 24, 2014, 12:48 a.m.
Do we also need to escape the path?	chipx86	June 24, 2014, 12:49 a.m.
Since type is a reserved word, we should probably use status or something.	chipx86	June 24, 2014, 12:49 a.m.
Can we use the ReviewRequest.* constants, like SUBMITTED?	chipx86	June 24, 2014, 12:50 a.m.
Should compare with BaseComment.OPEN.	chipx86	June 24, 2014, 12:51 a.m.
No blank line.	chipx86	June 24, 2014, 12:52 a.m.
Missing periods in the help text. Is there any URL we can link to for finding the webhook URL? The …	chipx86	June 24, 2014, 12:53 a.m.
We should use from reviewboard.extensions.packaging import setup here.	chipx86	June 24, 2014, 12:54 a.m.
Let's add the version stuff to rbslack/__init__.py, for consistency and for releases with the new build stuff.	chipx86	June 24, 2014, 12:56 a.m.
We should attribute this to Beanbag, like the other extensions have.	chipx86	June 24, 2014, 12:57 a.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	June 24, 2014, 1:10 a.m.
We should switch to "Slack" here and all other places where we refer to their name. The logo shows "#slack," …	chipx86	June 24, 2014, 3:04 p.m.
We should set released to False up until we do the release. Also, before we label this 1.0, I want …	chipx86	June 24, 2014, 3:05 p.m.
Should we make this a hidden setting (just an entry in default_settings)? We could then set it to "RBCommons" when …	chipx86	June 24, 2014, 3:05 p.m.
The wording feels odd. Maybe something like "The name of the channel review request updates are sent to."?	chipx86	June 24, 2014, 3:06 p.m.
No need for this anymore.	chipx86	June 24, 2014, 3:05 p.m.
We should also add classifiers to this. There's a brand new classifier we should also provide: Framework :: Review Board	chipx86	June 24, 2014, 3:07 p.m.
This should be "Slack".	chipx86	June 25, 2014, 5:13 p.m.

Tool: Pyflakes
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/extension.py
    rbslack/rbslack/forms.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING



Tool: PEP8 Style Checker
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/extension.py
    rbslack/rbslack/forms.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING

rbslack/rbslack/extension.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (99 > 79 characters)
```
rbslack/rbslack/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
rbslack/rbslack/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```

Change Summary:

Fix some line length issues.

Commit:

eeeb61a524941c475e20a8b97760799ec02a2a11

3fc5b3e8b3cb457a408b4075265c730146a5f864

Diff:

Revision 2 (+341)

Show changes

	rbslack/COPYING
	rbslack/README.md
	rbslack/setup.py
	rbslack/rbslack/admin_urls.py
	rbslack/rbslack/extension.py
	rbslack/rbslack/forms.py

Tool: PEP8 Style Checker
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/extension.py
    rbslack/rbslack/forms.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING



Tool: Pyflakes
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/extension.py
    rbslack/rbslack/forms.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING

rbslack/rbslack/extension.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (99 > 79 characters)
```

```
Can't wait to use this :D
```
rbslack/COPYING (Diff revision 2)
The issue has been resolved. Show all issues
```
Shouldn't we copyright Beanbag, Inc.?
```
rbslack/rbslack/extension.py (Diff revision 2)
The issue has been resolved. Show all issues
```
No blank line here.
```

rbslack/rbslack/extension.py (Diff revision 2)

The issue has been resolved. Show all issues

This is going to break as soon as we do an update that in any way modifies the file.

If we need to ship a stable logo image for this somewhere, I'd say let's just stick this in a reliable place in an S3 bucket. Probably the images.reviewboard.org bucket somewhere, maybe under a rbslack path so that we don't forget why it's there.

rbslack/rbslack/extension.py (Diff revision 2)

The issue has been resolved. Show all issues

You shouldn't need the default, since RB already defaults to http, and get() will fall back on that.

rbslack/rbslack/extension.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Do we also need to escape the path?
```
1. david June 24, 2014, 1 a.m.
  Path is part of the URL, so no.

rbslack/rbslack/extension.py (Diff revision 2)

The issue has been dropped. Show all issues

Since type is a reserved word, we should probably use status or something.

david June 24, 2014, 1 a.m.

I'd love to, but the signal uses 'type' as a kwarg.

rbslack/rbslack/extension.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Can we use the ReviewRequest.* constants, like SUBMITTED?
```
rbslack/rbslack/extension.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Should compare with BaseComment.OPEN.
```
rbslack/rbslack/forms.py (Diff revision 2)
The issue has been resolved. Show all issues
```
No blank line.
```

rbslack/rbslack/forms.py (Diff revision 2)

The issue has been resolved. Show all issues

Missing periods in the help text.

Is there any URL we can link to for finding the webhook URL? The account page somehow?

david June 24, 2014, 1 a.m.

Nothing that can be linked (It's something like "https://beanbag.slack.com/services/2412829182"). Even the account pages include their account name in the URL.

rbslack/setup.py (Diff revision 2)

The issue has been resolved. Show all issues

We should use from reviewboard.extensions.packaging import setup here.

rbslack/setup.py (Diff revision 2)

The issue has been resolved. Show all issues

Let's add the version stuff to rbslack/__init__.py, for consistency and for releases with the new build stuff.

rbslack/setup.py (Diff revision 2)

The issue has been resolved. Show all issues

We should attribute this to Beanbag, like the other extensions have.

Commit:

3fc5b3e8b3cb457a408b4075265c730146a5f864

f834f35cf80a2ca2818427f12f35f9d33286d190

Diff:

Revision 3 (+403)

Show changes

	rbslack/COPYING
	rbslack/README.md
	rbslack/setup.py
	rbslack/rbslack/__init__.py
	rbslack/rbslack/admin_urls.py
	rbslack/rbslack/extension.py
	rbslack/rbslack/forms.py

Tool: PEP8 Style Checker
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING



Tool: Pyflakes
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING

rbslack/rbslack/extension.py (Diff revision 3)

The issue has been resolved. Show all issues

Col: 80
 E501 line too long (91 > 79 characters)

chipx86 June 24, 2014, 1:05 a.m.

I just set things up so that http://images.reviewboard.org/rbslack/logo.png will work.

Commit:

f834f35cf80a2ca2818427f12f35f9d33286d190

87cb2107370811656def5b98be5b4fb71a7784b5

Diff:

Revision 4 (+403)

Show changes

	rbslack/COPYING
	rbslack/README.md
	rbslack/setup.py
	rbslack/rbslack/__init__.py
	rbslack/rbslack/admin_urls.py
	rbslack/rbslack/extension.py
	rbslack/rbslack/forms.py

Tool: PEP8 Style Checker
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING



Tool: Pyflakes
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING

rbslack/README.md (Diff revision 4)

The issue has been resolved. Show all issues

We should switch to "Slack" here and all other places where we refer to their name. The logo shows "#slack," but all text on their site referencing their service uses "Slack."

rbslack/rbslack/__init__.py (Diff revision 4)

The issue has been resolved. Show all issues

We should set released to False up until we do the release.

Also, before we label this 1.0, I want to spend some more time using this and testing it in production, and also figure out how we can make this all work well with RBCommons.

rbslack/rbslack/extension.py (Diff revision 4)

The issue has been resolved. Show all issues

Should we make this a hidden setting (just an entry in default_settings)? We could then set it to "RBCommons" when we get it working there.

rbslack/rbslack/forms.py (Diff revision 4)

The issue has been resolved. Show all issues

The wording feels odd. Maybe something like "The name of the channel review request updates are sent to."?

rbslack/setup.py (Diff revision 4)
The issue has been resolved. Show all issues
```
No need for this anymore.
```

rbslack/setup.py (Diff revision 4)

The issue has been resolved. Show all issues

We should also add classifiers to this.

There's a brand new classifier we should also provide:

Framework :: Review Board

Commit:

87cb2107370811656def5b98be5b4fb71a7784b5

e851f7b5949239a66f1897d38ed8b3507c91dcfc

Diff:

Revision 5 (+413)

Show changes

	rbslack/COPYING
	rbslack/README.md
	rbslack/setup.py
	rbslack/rbslack/__init__.py
	rbslack/rbslack/admin_urls.py
	rbslack/rbslack/extension.py
	rbslack/rbslack/forms.py

Tool: Pyflakes
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING



Tool: PEP8 Style Checker
Processed Files:
    rbslack/rbslack/admin_urls.py
    rbslack/rbslack/__init__.py
    rbslack/rbslack/forms.py
    rbslack/rbslack/extension.py
    rbslack/setup.py

Ignored Files:
    rbslack/README.md
    rbslack/COPYING

Ship it!

rbslack/README.md (Diff revision 5)
The issue has been resolved. Show all issues
```
This should be "Slack".
```

Status:: Completed
Change Summary:: Pushed to master (3af20cd).