Quick Survey: Automatically mark viewed files reviewed

[Ben Rohlfs]

Hi all,

Gerrit has a user setting "Automatically mark viewed files reviewed". It means that as soon as you view a file it will be marked as reviewed.

This even applies to when all files are expanded in the change view, which is causing lots of write QPS for large changes and might not work as expected, because it even applies to content that was never on-screen.

I was wondering if we could just remove the "Automatically mark viewed files reviewed" feature from the *change page*. WDYT? Who is actively using that feature and can provide some feedback? FWIW, a lot of users that I have talked to don't use the "mark files as reviewed" feature at all.

Thanks for your responses.

-Ben

[Clark Boylan]

When we did our last Gerrit server upgrade we made the decision to not
migrate the accountPatchReviewDB database as it simplified things
quite a bit. When we did this it reset all reviewed files to
unreviewed because they no longer had any DB records. If anyone
noticed they never complained. However, in that process we found that
the MariaDB JDBC connector didn't work with Gerrit and ended up
needing to fix that locally and upstream. In response to that we added
testing to our local Gerrit deployment CI to ensure that setting
reviewed and unreviewed works as expected. Basically a lot of work for
something no one really seems to notice.

That said, I personally make use of this feature occasionally. Gerrit
presents the list of files in alpha sorted order, and for many changes
reviewing in that order doesn't make much sense. The "reviewed" flag
can help keep track of what you have looked at in larger changes when
you are jumping from file to file out of order. I think I can live
without this functionality if it makes Gerrit more performant and our
CI simpler. I am curious to see if others rely on this and if there
are maybe other uses that I haven't considered though.

>
> -Ben
>

[Ben Rohlfs]

Thanks for the feedback, Clark!

Note that while the feature is generally questionable, I was just asking about *automatically* marking files as reviewed, i.e. the file gets marked as reviewed just by looking at it. Do you also use that aspect of the feature?

[Adrià Vilanova Martínez]

Hi! Replying below.

On Monday, January 24, 2022 at 6:11:09 PM UTC+1 bro...@google.com wrote:

Thanks for the feedback, Clark!

Note that while the feature is generally questionable, I was just asking about *automatically* marking files as reviewed, i.e. the file gets marked as reviewed just by looking at it. Do you also use that aspect of the feature?

I'm curious as to why it is "generally questionable". In my case, I find it very useful since I start by reviewing trivial changes to files and then take a look at the remaining unreviewed files and iteratively choose one to review until there are none left.

I'm one of the apparently few users of the "Automatically mark viewed files reviewed" feature.

I'll explain a little bit what my workflow is to review changes: I only use the change screen, and rarely go inside the single file change view. When I want to review a file, I expand it and begin reviewing it; then, when I finish reviewing it, I just close it and go on to the next one. If I have to look up another file I just open the other one and either review it completely or click the "mark unreviewed" button and close it.

I'll disable this feature and see how I adapt over the next few weeks :)

If you don't have any input from Chromium developers, maybe it would be worth sharing this thread at https://groups.google.com/a/chromium.org/g/chromium-discuss?

Cheers :-) 

 

>
> -Ben
>

[Åsmund Østvold]

Hi,

On Mon, Jan 24, 2022, 19:16 Adrià Vilanova Martínez <jocde...@gmail.com> wrote:

Hi! Replying below.

On Monday, January 24, 2022 at 6:11:09 PM UTC+1 bro...@google.com wrote:

Thanks for the feedback, Clark!

Note that while the feature is generally questionable, I was just asking about *automatically* marking files as reviewed, i.e. the file gets marked as reviewed just by looking at it. Do you also use that aspect of the feature?

I'm curious as to why it is "generally questionable". In my case, I find it very useful since I start by reviewing trivial changes to files and then take a look at the remaining unreviewed files and iteratively choose one to review until there are none left.

I work like this too, and I know several of my coworkers also work like this. I would miss this feature. 

Regards,

Åsmund

[Martin Fick]

On 2022-01-24 09:47, 'Ben Rohlfs' via Repo and Gerrit Discussion wrote:
>
> Gerrit has a user setting "Automatically mark viewed files reviewed".
> It means that as soon as you view a file it will be marked as
> reviewed.
>
> This even applies to when all files are expanded in the change view,
> which is causing lots of write QPS for large changes and might not
> work as expected, because it even applies to content that was never
> on-screen.

Yes, I personally think this is a problematic approach, so I don't
use it. However, I suspect many people might think the same thing
about automatically marking emails as "read" when first displaying
them. Likely because both use cases are useful, most email clients
make this optional.

> I was wondering if we could just remove the "Automatically mark viewed
> files reviewed" feature from the *change page*. WDYT? Who is actively
> using that feature and can provide some feedback?

I suspect it would be possible to "mine" your users' settings to
see how many users have this feature enabled. If more than 0 users
have it enabled, I would assume it is useful. It has been a long
time, but I seem to recall that Shawn once said that this actually
an internal Google requirement to have this feature when he first
implemented it. It may be worth investigating if that was true,
and whether it still is.

If it would be valuable to you, we may be able to mine our users
to see if any of them use that setting.

> FWIW, a lot of users that I have talked to don't use the
> "mark files as reviewed" feature at all.

I use this feature EVERYTIME I review a change. However, since the
current WUI seems to have eliminated the visual UI elements to toggle
this while reviewing files, I am not surprised that many users would
not use it (it is essentially a hidden feature). The way that I use
it is via the capital "M" shortcut which not only marks a file
"reviewed", but also advances to the next "unreviewed" file! Since I
find this to be an amazingly useful feature, my hope is that at some
point the WUI will re-expose this as something that can be done via
the mouse in a convenient way so that it is a more discoverable
feature for newer users.

-Martin

--
The Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation

[Björn Pedersen]

Hi,

I really use this a lot ( our code base often means you look at changes quite out of order). I seldom use the show all-files mode, where I think marking it reviewed automatically should probably only happen after scrolling to the end, or when the end of a file is on screen. Making it required manual in this display mode would be  acceptable for personally as  well.

Björn

[Nasser Grainawi]

On Jan 24, 2022, at 9:47 AM, 'Ben Rohlfs' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

Hi all,

Gerrit has a user setting "Automatically mark viewed files reviewed". It means that as soon as you view a file it will be marked as reviewed.

I think this is the default, isn’t it?

This even applies to when all files are expanded in the change view, which is causing lots of write QPS for large changes and might not work as expected, because it even applies to content that was never on-screen.

It does seem strange that it marks things you don’t see as reviewed. I think the community would welcome solutions that improve that.

I was wondering if we could just remove the "Automatically mark viewed files reviewed" feature from the *change page*. WDYT? Who is actively using that feature and can provide some feedback?

I use the Expand All feature for changes with small diffs (even if there are several files), but I don’t typically rely on the Reviewed status in that case.

I do use the Reviewed status when individually expanding diffs from the change page, because then I’m often looking at a change with several small diffs that I want to see right away and then I click through into the diff screen for files with larger diffs. There I appreciate that I can click Next and it will skip the small diffs I've already looked at.

FWIW, a lot of users that I have talked to don't use the "mark files as reviewed" feature at all.

FYI, this comment confused me until I re-read your email a couple times. It made me think you wanted to remove the automatic reviewed marking feature entirely and not just for when expanding diffs from the change page.

Nasser

Thanks for your responses.

-Ben

--
--
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.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAEWS%2BaNwCs4oythicVcf4FKqJ%2Bi8hMa4OH46h%3DK%2ByF-KRRVfug%40mail.gmail.com.

[Edwin Kempin]

On Wed, Jan 26, 2022 at 1:17 AM Nasser Grainawi <nas...@codeaurora.org> wrote:

On Jan 24, 2022, at 9:47 AM, 'Ben Rohlfs' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

Hi all,

Gerrit has a user setting "Automatically mark viewed files reviewed". It means that as soon as you view a file it will be marked as reviewed.

I think this is the default, isn’t it?

Yes, this should be the default:

https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java#72

 

To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/E030B8C5-66B1-4D5E-9067-C209D7AE34F8%40codeaurora.org.

[Ben Rohlfs]

Thanks for all your feedback! That helped a lot with understanding the usage of the "mark files reviewed" feature.

Sorry for creating confusion. We do not intend to remove the "mark files reviewed" feature. And apparently automatically marking files as reviewed is a setting that also some users like very much. So we will keep that, as well.

It would be nice, if the setting's default value was false. That would enable us to better understand the usage, but I will probably not look into this more.

For the change page I would propose that "Expand All" just never marks files as reviewed. Only individually expanded files will be marked as reviewed. That seems to be an uncontroversial improvement.

Thanks, Ben

[Sven Selberg]

On Wednesday, January 26, 2022 at 9:02:42 AM UTC+1 bro...@google.com wrote:

Thanks for all your feedback! That helped a lot with understanding the usage of the "mark files reviewed" feature.

Sorry for creating confusion. We do not intend to remove the "mark files reviewed" feature. And apparently automatically marking files as reviewed is a setting that also some users like very much. So we will keep that, as well.

It would be nice, if the setting's default value was false. That would enable us to better understand the usage, but I will probably not look into this more.

I like this idea.
It would lessen the impact to some (probably significant) degree since users that don't use the feature won't enable it.
Whereas those that are depending on the feature are able to change the default behavior, and (as you pointed out) by doing so

they give an indication of how used the feature is.

I think most user would buy the explanation that the default is changed due to performance issues, that it's unnecessary to

let the system spend time and resources when it isn't used. A good description in the release-notes and announcing it on

respective sites should keep the disgruntlement to a minimum (FLW).

Only my 5 cents.

/Sven

[Ben Rohlfs]

FYI sent https://gerrit-review.googlesource.com/c/gerrit/+/329782 for review. Clicking "Expand All" will not mark files as reviewed anymore.

-Ben