Shelving and Checkpointing support log messages

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Shelving and Checkpointing support log messages

Julian Foad-5
I had some feedback that the ability to give a short description or log
message seemed important, so I have added that feature on the prototype
branches.

$ svn shelve --help
...
  1. Save the local changes in the given PATHs to a patch file, and
     revert those changes from the WC. If a log message is given with
     '-m' or '-F', include it at the beginning of the patch file.
...

and similarly for the two designs of 'svn checkpoint'.

The respective 'list' functions then display the message (currently the
first line of it) of each patch or checkpoint.

I think we will encounter two distinct styles of usage:

  * "just put whatever I have aside, I haven't time to think about it
now, I'll come back to it later" -- no description or just a very brief
hint, and perhaps not even a name;

  * "this is a patch I have prepared" with a carefully written log message.

One rough edge is that "unshelving" a patch or finishing/squashing a
checkpoint series simply discards the log message(s). I welcome your
thoughts.

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: Shelving and Checkpointing support log messages

Daniel Shahaf-2
Julian Foad wrote on Thu, 24 Aug 2017 19:46 +0100:
> I think we will encounter two distinct styles of usage:
>
>   * "just put whatever I have aside, I haven't time to think about it
> now, I'll come back to it later" -- no description or just a very brief
> hint, and perhaps not even a name;
>
>   * "this is a patch I have prepared" with a carefully written log message.
>

Agreed.

> One rough edge is that "unshelving" a patch or finishing/squashing a
> checkpoint series simply discards the log message(s). I welcome your
> thoughts.

I doubt anyone would store a verbose long message on a shelved
patch if applying the patch would discard the log message.  People would
grow the habit of storing the log message out of band.

Conversely, to enable the "long log message" use-case, there should be an easy
"upgrade path" for the shelf's log message to become a commit log message;
optionally with editing first.

Example workflow:

1. There exists a shelf with a verbose log message
2. Unshelve that patch
3. 'make check'
4. 'svn commit'

In this workflow, it would be nice for $EDITOR to be pre-filled with the
shelf's log message.

The catch is, what happens if the patch is reverted (with 'svn revert'
or with an equivalent '/usr/bin/patch -R') before step 4.  In that case,
the user wouldn't want the shelf's message to be presupplied.

The problem is that between #2 and #4 we have hidden state: step #4
doesn't know whether it's committing the patch that had been
unshelved, or something else --- and if we have to guess, we'll
sometimes guess wrongly.

(That's all I have for now...)

Cheers,

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: Shelving and Checkpointing support log messages

Julian Foad-5
Daniel Shahaf wrote:

> Julian Foad wrote on Thu, 24 Aug 2017 19:46 +0100:
>> One rough edge is that "unshelving" a patch or finishing/squashing a
>> checkpoint series simply discards the log message(s). [...]
>
> [...] to enable the "long log message" use-case, there should be an easy
> "upgrade path" for the shelf's log message to become a commit log message;
> optionally with editing first.
>
> Example workflow:
>
> 1. There exists a shelf with a verbose log message
> 2. Unshelve that patch
> 3. 'make check'
> 4. 'svn commit'
>
> In this workflow, it would be nice for $EDITOR to be pre-filled with the
> shelf's log message.
>
> The catch is, what happens if the patch is reverted [...] step #4
> doesn't know whether it's committing the patch that had been
> unshelved, or something else [...]

A neat solution could be to integrate Shelving with Changelists in such
a way that Unshelving means converting a patch to a changelist, and a
changelist grows support for storing a log message.

This would be a two-way integration: shelving a changelist would then
mean converting the changelist to a patch and reverting the changelist
from the WC.

This would be more in line with how, for example, Perforce defines a
changelist: there it is a change, whether committed or shelved or in one
or two other states; and includes a log message and some other metadata.

(In fact, right from when we introduced "changelists", I have wanted
them to support storing a log message, without even thinking about the
possibility of shelving.)

Example workflow:

1. There exists a shelved patch 'foo' with a verbose log message
2. 'svn unshelve foo'
3. 'make check'
4. 'svn commit --changelist foo'

In this example, the unshelve creates & populates changelist 'foo', and
the commit takes its log msg from changelist foo.

To me this seems very appealing.

I outlined this idea in the section "Integrate Shelving with
Changelists" in the "Shelving-Checkpointing Dev" doc:
https://docs.google.com/document/d/1PVgw0BdPF7v67oxIK7B_Yjmr3p28ojabP5N1PfZTsHk/edit#heading=h.ay5h5pfumpv8

What do you think?

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: Shelving and Checkpointing support log messages

Johan Corveleyn-3
On Fri, Aug 25, 2017 at 11:08 AM, Julian Foad <[hidden email]> wrote:

> Daniel Shahaf wrote:
>> Julian Foad wrote on Thu, 24 Aug 2017 19:46 +0100:
>>> One rough edge is that "unshelving" a patch or finishing/squashing a
>>> checkpoint series simply discards the log message(s). [...]
>>
>> [...] to enable the "long log message" use-case, there should be an easy
>> "upgrade path" for the shelf's log message to become a commit log message;
>> optionally with editing first.
>>
>> Example workflow:
>>
>> 1. There exists a shelf with a verbose log message
>> 2. Unshelve that patch
>> 3. 'make check'
>> 4. 'svn commit'
>>
>> In this workflow, it would be nice for $EDITOR to be pre-filled with the
>> shelf's log message.
>>
>> The catch is, what happens if the patch is reverted [...] step #4
>> doesn't know whether it's committing the patch that had been
>> unshelved, or something else [...]
>
> A neat solution could be to integrate Shelving with Changelists in such
> a way that Unshelving means converting a patch to a changelist, and a
> changelist grows support for storing a log message.
>
> This would be a two-way integration: shelving a changelist would then
> mean converting the changelist to a patch and reverting the changelist
> from the WC.
>
> This would be more in line with how, for example, Perforce defines a
> changelist: there it is a change, whether committed or shelved or in one
> or two other states; and includes a log message and some other metadata.
>
> (In fact, right from when we introduced "changelists", I have wanted
> them to support storing a log message, without even thinking about the
> possibility of shelving.)
>
> Example workflow:
>
> 1. There exists a shelved patch 'foo' with a verbose log message
> 2. 'svn unshelve foo'
> 3. 'make check'
> 4. 'svn commit --changelist foo'
>
> In this example, the unshelve creates & populates changelist 'foo', and
> the commit takes its log msg from changelist foo.
>
> To me this seems very appealing.
>
> I outlined this idea in the section "Integrate Shelving with
> Changelists" in the "Shelving-Checkpointing Dev" doc:
> https://docs.google.com/document/d/1PVgw0BdPF7v67oxIK7B_Yjmr3p28ojabP5N1PfZTsHk/edit#heading=h.ay5h5pfumpv8
>
> What do you think?

+1, that looks like a good approach.

In the google doc you mentioned some issues with this:

* a changelist can include unmodified paths
  should a shelved patch also include unmodified paths?

-> I guess that would be the best solution for this. The shelve should
keep (in metadata) a list of paths that were part of the changelist.

* a path can appear in multiple shelves, but only in one changelist
  what to do when unshelving if a path clashes?

-> Hmm, that's a hard one. No idea for now, except perhaps extending
changelist to make it possible for paths to be part of multiple
changelists too.

FWIW: IntelliJ IDEA gives you a warning when you start editing a file
that's part of some changelist, while your default changelist is set
to another changelist ("File from non-active changelist is modified").
The user can "Move changes", "Switch changelist" or "Ignore" (though
it's not clear to me what "Ignore" does ... the file doesn't become
part of the "active changelist" suddenly, or of both, so this seems
equal to "Move changes").

* any extensions should apply uniformly to both shelving and changelists
  if a shelved change supports a log msg, a changelist should support
a log msg too; this would be a good enhancement for changelists anyway
even without shelving

-> Yup, sounds good to me :)


I see one more important caveat: changelists currently don't support
directories as members.

I have some more thoughts on shelves and checkpoints, but I'll put
them in a separate thread.

--
Johan
Reply | Threaded
Open this post in threaded view
|

Re: Shelving and Checkpointing support log messages

Stefan Sperling
In reply to this post by Julian Foad-5
On Fri, Aug 25, 2017 at 10:08:04AM +0100, Julian Foad wrote:
> Example workflow:
>
> 1. There exists a shelved patch 'foo' with a verbose log message
> 2. 'svn unshelve foo'

There can only be one changelist per file.
What if at this point there is another changelist 'bar' which includes a
file modified by the shelved patch? Would 'unshelve' error out or change
that file's changelist to 'foo'?

I suppose it should error out and ask the user to commit or remove
the conflicting changelist first.

> 3. 'make check'
> 4. 'svn commit --changelist foo'
>
> In this example, the unshelve creates & populates changelist 'foo', and
> the commit takes its log msg from changelist foo.
>
> To me this seems very appealing.

Yes, indeed!