New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include branch name (and commit counts?) on Minimal push notifications #19

Closed
bloveridge opened this Issue Sep 14, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@bloveridge

bloveridge commented Sep 14, 2015

Problem

We use a lot of feature branches and do a lot of rebases in our workflow, and we like to have Stash notifications come into our general development room alongside our regular dev chatter. Unfortunately, we haven't been able to find a good middle ground with the current options for Push notifications.

  1. Verbose—yeah, we expected this one to be verbose, and it sure is! When doing a rebase, this also brought over comments on the various commits that suddenly were in the new branch history. Way too noisy, so moved down to Compact.
  2. Compact—had really high hopes for this one. During regular development this is great, because it shows the author, branch, SHA1, and summary.
    • During a rebase, however, this also gets really noisy. It adds a line for each and every commit that was added on the branch being rebased atop. If your branch is dozens (or, heaven forbid, hundreds) of commits behind then this quickly spams your slack room and is painful.
    • So we decided to try Minimal.
  3. Minimal—unfortunately, this is just too minimal to be useful to us. All it shows is "Push on [repo] by [author]." There is no differentiation between whether it was pushed to master or a feature branch, and it isn't useful for us to find out "hey, somebody just pushed something somewhere."

Proposal

Change Minimal to be just a tiny bit less minimal, displaying at the very least the branch that was pushed to, and ideally the number of commits.

Example: current behavior

Stash: Push on insight by Ben Loveridge <email>. See commit list.
Stash: Push on insight by Ben Loveridge <email>. See commit list.

Example: after

Stash: Push on insight : master by Ben Loveridge <email>. See commit list.
Stash: Push on insight : feature/the_new_thing by Ben Loveridge <email>. See commit list.

Example: after (alternate)

Stash: Push on insight : master by Ben Loveridge <email> (1 commit). See commit list.
Stash: Push on insight : feature/the_new_thing by Ben Loveridge <email> (3 commits). See commit list.

@bloveridge

This comment has been minimized.

bloveridge commented Sep 14, 2015

All in all, thanks very much for your plugin, we are putting it to good use, and it is nice that it supports PR operations that the official Stash -> HipChat integration supports (specifically PR approval).

@marco-roy

This comment has been minimized.

marco-roy commented Sep 15, 2015

+1

taxilian added a commit to gradecam/stash2slack that referenced this issue Sep 18, 2015

Refactor the push listener to add a lot of flexibility
- Fixes pragbits#19 - adds commit count
- Fixes pragbits#20 - Differentiates tag pushes
- Adds links in other parts of the messages
- Reformats commit hashes on compact mode to make messages left aligned
- Fixes links for branches to go to the correct branch
- Add the project name to the repo name in messages
- Combine all messages into one attachment to allow slack's auto collapse features to work (compact mode only)

taxilian added a commit to gradecam/stash2slack that referenced this issue Sep 18, 2015

Refactor the push listener to add a lot of flexibility
- Fixes pragbits#19 - adds commit count
- Fixes pragbits#20 - Differentiates tag pushes
- Adds links in other parts of the messages
- Reformats commit hashes on compact mode to make messages left aligned
- Fixes links for branches to go to the correct branch
- Add the project name to the repo name in messages
- Combine all messages into one attachment to allow slack's auto collapse features to work (compact mode only)

@pragbits pragbits closed this in #21 Sep 21, 2015

dszabo added a commit that referenced this issue Sep 21, 2015

Merge branch 'feature/taxilian'
Fix #19, #20, and a bunch of related cleanups and improvements to push hook #21
Implemented #17
@marco-roy

This comment has been minimized.

marco-roy commented Sep 25, 2015

Looks like the "alternate" example has been implemented in 1.7. Great work guys!

Push on SuchRepo branch master by Very Quality <very@quality.com> (1 commit). See commit list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment