WIP and watches

36 views
Skip to first unread message

Logan Hanks

unread,
May 24, 2017, 5:44:12 PM5/24/17
to Repo and Gerrit Discussion
I'm working on applying WIP to notifications (code in progress). One requirement is to have a way for a watcher to observe everything that happens only on changes that have reached a point of starting review. I was thinking of adding a new notify type, called "review_started_changes", to provide that.

However, we may want to consider just modifying the existing notify types to not receive notifications on "fresh" WIP changes. That is, if a change is created in the WIP state, we could just default the notify level below ALL (I'm thinking OWNER) on all notifications. This is, in fact, what I've proposed all along, but I want to make sure this doesn't surprise anyone depending on watching new_changes, new_patchsets, etc.

The upside:

1. A user can upload a change, never start review, and never trigger an email about it.
2. We have the option to avoid adding a new, potentially confusing notify type.

The downside:

1. Watchers no longer have a guarantee that they are seeing *every* change. That may be ok for a project, because WIP changes can't be merged without starting review and triggering notifications. But, eager watchers would miss out on early notification that a change is coming, and a malicious user may be able to upload content without immediate observation.

Would anyone be unpleasantly surprised by missing out on notifications of never-reviewed WIP changes? If not, I think that may provide a better user experience.

Jonathan Nieder

unread,
May 24, 2017, 6:50:43 PM5/24/17
to Logan Hanks, Repo and Gerrit Discussion
> However, we may want to consider just modifying the existing notify types to not receive notifications on "fresh" WIP changes.

Yes, please. Someone uploading a fresh WIP change is a private event. I noticed recently that when such a change is abandoned without starting review, I get notifications about it. The change you're proposing would make it so that I don't have to notice such a change at all.

What I'm starting to wonder about is whether there should be a watch type <https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#project-watch-info> for people who want to see all events, including events for fresh WIP changes (and including changes made with notify=NONE). I would expect that to take care of those unusual use cases where some system really wants an email for every event.

Thoughts?
Jonathan

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Matthias Sohn

unread,
May 25, 2017, 4:30:00 AM5/25/17
to Jonathan Nieder, Logan Hanks, Repo and Gerrit Discussion
On Thu, May 25, 2017 at 12:50 AM, 'Jonathan Nieder' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:
> However, we may want to consider just modifying the existing notify types to not receive notifications on "fresh" WIP changes.

Yes, please. Someone uploading a fresh WIP change is a private event. I noticed recently that when such a change is abandoned without starting review, I get notifications about it. The change you're proposing would make it so that I don't have to notice such a change at all.

What I'm starting to wonder about is whether there should be a watch type <https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#project-watch-info> for people who want to see all events, including events for fresh WIP changes (and including changes made with notify=NONE). I would expect that to take care of those unusual use cases where some system really wants an email for every event.

we feed the complete event stream into our monitoring system as one means to monitor usage of the system.
For this use case we'd like to see all event types. 

-Matthias

Thoughts?
Jonathan

Le mer. 24 mai 2017 à 14:44, 'Logan Hanks' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> a écrit :
I'm working on applying WIP to notifications (code in progress). One requirement is to have a way for a watcher to observe everything that happens only on changes that have reached a point of starting review. I was thinking of adding a new notify type, called "review_started_changes", to provide that.

However, we may want to consider just modifying the existing notify types to not receive notifications on "fresh" WIP changes. That is, if a change is created in the WIP state, we could just default the notify level below ALL (I'm thinking OWNER) on all notifications. This is, in fact, what I've proposed all along, but I want to make sure this doesn't surprise anyone depending on watching new_changes, new_patchsets, etc.

The upside:

1. A user can upload a change, never start review, and never trigger an email about it.
2. We have the option to avoid adding a new, potentially confusing notify type.

The downside:

1. Watchers no longer have a guarantee that they are seeing *every* change. That may be ok for a project, because WIP changes can't be merged without starting review and triggering notifications. But, eager watchers would miss out on early notification that a change is coming, and a malicious user may be able to upload content without immediate observation.

Would anyone be unpleasantly surprised by missing out on notifications of never-reviewed WIP changes? If not, I think that may provide a better user experience.

--
--

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

Logan Hanks

unread,
May 25, 2017, 1:24:40 PM5/25/17
to Matthias Sohn, Jonathan Nieder, Repo and Gerrit Discussion
A few questions for those of you who depend on a firehose of notifications on all actions on all changes:

1. How are you configuring this? Is it user-level or project-level? Which watch type(s) do you use? (https://gerrit-review.googlesource.com/Documentation/user-notify.html)

2. Do you care that some notifications may be hidden from you (e.g. by passing notify=NONE)?

Dave Borowitz

unread,
May 25, 2017, 1:33:38 PM5/25/17
to Logan Hanks, Matthias Sohn, Jonathan Nieder, Repo and Gerrit Discussion
I think there might be some confusion. When Logan says "notifications" he means "emails", which are (AFAIK) the only thing controlled by the notify preferences and REST API arguments.

It sounds like Matthias is talking about the *Listener extension points and/or stream-events, which is unaffected by Logan's proposal.

Logan Hanks

unread,
May 25, 2017, 1:36:49 PM5/25/17
to Dave Borowitz, Matthias Sohn, Jonathan Nieder, Repo and Gerrit Discussion
Yeah, I'm talking only about emails, I should've said that instead of "notifications".

Does anyone count on a complete firehose of notification emails?

Matthias Sohn

unread,
May 26, 2017, 5:49:56 AM5/26/17
to Dave Borowitz, Logan Hanks, Jonathan Nieder, Repo and Gerrit Discussion
On Thu, May 25, 2017 at 7:33 PM, Dave Borowitz <dbor...@google.com> wrote:
I think there might be some confusion. When Logan says "notifications" he means "emails", which are (AFAIK) the only thing controlled by the notify preferences and REST API arguments.

It sounds like Matthias is talking about the *Listener extension points and/or stream-events, which is unaffected by Logan's proposal.


you are right, we don't use emails for that but directly consume the complete event stream using the ssh stream-events
Reply all
Reply to author
Forward
0 new messages