Patch set terminology

[Alan Evangelista]

The usual meaning of a patch set is a set of patches. Gerrit uses this alternative meaning of change revision (aka change version) ? Why? This is a little confusing...

[Alan Evangelista]

AV> The usual meaning of a patch set is a set of patches. Gerrit uses this alternative meaning of change revision (aka change version) ? Why? This is a little confusing...

Reading the code, I now think this confusing terminology comes from there, as "Patch" class represents a change in a file, "PatchSet" represents a change in several files (a group of Patches) and "Change" represents a group of PatchSets and some metadata. This patch definition also is different from usual patch meaning (software update) and it is a little confusing imho. imho FilePatch would make more sense than Patch and Patch would make more sense than PatchSet as classes names. In the UI, I still think that "revision" or "version" are the more adequate terms to replace the current "PatchSet". Are the developers reading this group and they think this makes sense? I'd like to have some feedback before start implementing and submitting a patch. Should I open an issue in Gerrit issue tracker to have that?

[Martin Fick]

On Sunday, April 26, 2015 08:56:16 PM Alan Evangelista
wrote:

> AV> The usual meaning of a patch set is a set of patches.
> Gerrit uses this alternative meaning of change revision
> (aka change version) ? Why? This is a little confusing...
>
> Reading the code, I now think this confusing terminology
> comes from there, as "Patch" class represents a change in
> a file, "PatchSet" represents a change in several files
> (a group of Patches) and "Change" represents a group of
> PatchSets and some metadata. This patch definition also
> is different from usual patch meaning (software update)
> and it is a little confusing imho.

I don't think the term patch to represent a diff (something
that can be fed to the "patch" command) is unusual in the
source code world.

> imho FilePatch would
> make more sense than Patch and Patch would make more
> sense than PatchSet as classes names. In the UI, I still
> think that "revision" or "version" are the more adequate
> terms to replace the current "PatchSet".

I don't think "revision' would "version" correlate to a
patchset, but rather to a "patchset number" or a "patchset
revision", maybe I don't understand what you are suggesting
here?

> Are the
> developers reading this group and they think this makes
> sense? I'd like to have some feedback before start
> implementing and submitting a patch. Should I open an
> issue in Gerrit issue tracker to have that?

I agree that the wording could be improved potentially, but
I would find it too intrusive to desire such changes to the
code base at this point. I also suspect that it would be
very difficult to come up with better terms that make sense to
most people. This is not a bike-shedding discussion that I
personally think would add a lot of value at this point to
Gerrit,

-Martin

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

[Justin Clift]

On 27 Apr 2015, at 17:22, Martin Fick <mf...@codeaurora.org> wrote:
<snip>

> This is not a bike-shedding discussion that I
> personally think would add a lot of value at this point to
> Gerrit,

Yeah. There is a large Gerrit userbase which has learned and
uses the existing terminology. Adjusting it now doesn't sound
like a win. :/

+ Justin

--
GlusterFS - http://www.gluster.org

An open source, distributed file system scaling to several
petabytes, and handling thousands of clients.

My personal twitter: twitter.com/realjustinclift

[Alan Evangelista]

I don't think the term patch to represent a diff (something
that can be fed to the "patch" command) is unusual in the
source code world.

In the source code world, a diff (and a patch) usually is not restricted to a single file. At least that's what I see in most open development communities and in my work environment. To make this point stronger, the patch utility in Linux you just mentioned may apply a diff to several files at once.

Renaming Patch class to FilePatch and PatchSet class to Patch (or FilesPatch is you want to be explicit) would be clearer for beginner developers imho.

 

I don't think "revision' would "version" correlate to a
patchset, but rather to a "patchset number" or a "patchset
revision", maybe I don't understand what you are suggesting
here?

Each time a Gerrit change is resubmitted, a new patch set is created.
I can check the code of each patch set separately and compare different patch sets.
It seems to me that Gerrit patch set correlates exactly to a "change revision"
or "change version". Am I losing something?
 

I agree that the wording could be improved potentially, but
I would find it too intrusive to desire such changes to the
code base at this point.

I have suggested 2 changes: one in the code and one in the UI.
The first will only be seen by development community, the second
will be seen by users. You meant both here?

[Martin Fick]

On Monday, April 27, 2015 10:02:53 AM Alan Evangelista
wrote:

>
> > I agree that the wording could be improved potentially,
> > but
> > I would find it too intrusive to desire such changes to
> > the code base at this point.
>
> I have suggested 2 changes: one in the code and one in the
> UI. The first will only be seen by development community,
> the second will be seen by users. You meant both here?

Yes.