[PATCH] doc: clarify that notes can be attached to any type of stored object

[PATCH] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-notes.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8de3499..5375d98 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -234,8 +234,9 @@ which operation triggered the update, and the commit authorship is
 determined according to the usual rules (see linkgit:git-commit[1]).
 These details may change in the future.
 
-It is also permitted for a notes ref to point directly to a tree
-object, in which case the history of the notes can be read with
+It is also permitted for a notes ref to point to any other object in
+the object store besides commit objects, that is annotated tags, blobs
+or trees. For the latter, the history of the notes can be read with
 `git log -p -g <refname>`.
 
 
-- 
2.8.0.windows.1

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  Documentation/git-notes.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 8de3499..5375d98 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -234,8 +234,9 @@ which operation triggered the update, and the commit authorship is
>  determined according to the usual rules (see linkgit:git-commit[1]).
>  These details may change in the future.
>  
> -It is also permitted for a notes ref to point directly to a tree
> -object, in which case the history of the notes can be read with
> +It is also permitted for a notes ref to point to any other object in
> +the object store besides commit objects, that is annotated tags, blobs
> +or trees. For the latter, the history of the notes can be read with
>  `git log -p -g <refname>`.

I do not think this is correct place to patch.  The original is not
talking about what objects can have notes attached at all.  What it
explains is this.

    <refname> aka refs/notes/<name> (where <name> typically is
    "commit") is usually a commit, whose tree is a notes-shaped
    tree.  The (normal) history you get by following the parent link
    of the commit represents how the entire set of notes evolved.
    However, it is OK for the <refname> to point directly to a tree,
    which is a notes-shaped one, without an enclosing commit.  You
    would lose the normal way to learn how the entire set of notes
    evolved, but you could do "git log -p -g <refname>", i.e. by
    following its reflog, to pretend as if the history is recorded.

There is no way a blob can be pointed by <refname> there and expect
it to work sensibly at all.

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Sebastian Schuberth <sschuberth@gmail.com> writes:
>
>> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
>> ---
>>  Documentation/git-notes.txt | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
> I do not think this is correct place to patch.

So,... where is the right place?

Let's disect what we have in the DESCRIPTION section.

    DESCRIPTION
    -----------
    Adds, removes, or reads notes attached to objects, without touching
    the objects themselves.

This says "notes attached to objects" and "the objects themselves".
They do not limit the target of attaching a note to "commits".
So this may be the place to add "  Note that notes can be attached
to any kind of objects, not limited to commits" or something, if
we really wanted to.

    By default, notes are saved to and read from `refs/notes/commits`, but
    this default can be overridden.  See the OPTIONS, CONFIGURATION, and
    ENVIRONMENT sections below.  If this ref does not exist, it will be
    quietly created when it is first needed to store a note.

    A typical use of notes is to supplement a commit message without
    changing the commit itself. Notes can be shown by 'git log' along with
    the original commit message. To distinguish these notes from the
    message stored in the commit object, the notes are indented like the
    message, after an unindented line saying "Notes (<refname>):" (or
    "Notes:" for `refs/notes/commits`).

    Notes can also be added to patches prepared with `git format-patch` by
    using the `--notes` option. Such notes are added as a patch commentary
    after a three dash separator line.

This paragraph _might_ be confusing to new readers.  The "added to"
sounds as if you are attaching a note to a non-object which is a
patch.  But this "add" is about "inserting the contents of the note
attached to the commit being formatted" and corresponds to "can be
shown by" in the previous paragraph.  We may want to avoid the verb
"add" when talking about the use of data stored in an existing note
to somewhere else like this.

    To change which notes are shown by 'git log', see the
    "notes.displayRef" configuration in linkgit:git-log[1].

    See the "notes.rewrite.<command>" configuration for a way to carry
    notes across commands that rewrite commits.



    SUBCOMMANDS
    -----------

    list::
            List the notes object for a given object. If no object is
            given, show a list of all note objects and the objects they
            annotate (in the format "<note object> <annotated object>").
            This is the default subcommand if no subcommand is given.

    add::
            Add notes for a given object (defaults to HEAD). Abort if the

And this "Add notes for " should probably be reworded to "Attach
notes to" to match the first sentence in the description.

            object already has notes (use `-f` to overwrite existing notes).
            However, if you're using `add` interactively (using an editor
            to supply the notes contents), then - instead of aborting -
            the existing notes will be opened in the editor (like the `edit`
            subcommand).

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

On Fri, Apr 1, 2016 at 6:47 PM, Junio C Hamano <gitster@pobox.com> wrote:

>     DESCRIPTION
>     -----------
>     Adds, removes, or reads notes attached to objects, without touching
>     the objects themselves.
>
> This says "notes attached to objects" and "the objects themselves".
> They do not limit the target of attaching a note to "commits".
> So this may be the place to add "  Note that notes can be attached
> to any kind of objects, not limited to commits" or something, if
> we really wanted to.

Done, I'll send a patch shortly. But I wanted to list the supported
object types explicitly, in particular as many guide in the net are
unclear that only annotated tags are object, and lightweight ones are
not.

>     Notes can also be added to patches prepared with `git format-patch` by
>     using the `--notes` option. Such notes are added as a patch commentary
>     after a three dash separator line.
>
> This paragraph _might_ be confusing to new readers.  The "added to"
> sounds as if you are attaching a note to a non-object which is a
> patch.  But this "add" is about "inserting the contents of the note
> attached to the commit being formatted" and corresponds to "can be
> shown by" in the previous paragraph.  We may want to avoid the verb
> "add" when talking about the use of data stored in an existing note
> to somewhere else like this.

Done.

>     add::
>             Add notes for a given object (defaults to HEAD). Abort if the
>
> And this "Add notes for " should probably be reworded to "Attach
> notes to" to match the first sentence in the description.

After a bit of thinking, I don't believe we should do this. All
subcommand docs start with the verb the subcommand is named after. In
that sense making the "add" docs start with "Attach" would be
inconsistent and probably raise the question why the subcommand is not
called "attach" after all. Also, in the description it says "Adds,
removes, or reads notes attached to objects", so it includes "[...]
removes [...] notes attached to objects", and if you read it like this
the word "attach" is not specific to the "add" subcommand. So I left
this as-is in my patch.

-- 
Sebastian Schuberth

[PATCH] doc: Clarify which objects notes can be attached to

[Sebastian Schuberth]

Explicitly name the supported object types, and ensure patches cannot be
misinterpreted as non-objects that can have notes attached.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-notes.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8de3499..101e6ba 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -25,7 +25,8 @@ SYNOPSIS
 DESCRIPTION
 -----------
 Adds, removes, or reads notes attached to objects, without touching
-the objects themselves.
+the objects themselves.  Supported objects are commits, blobs, trees
+and annotated tags.
 
 By default, notes are saved to and read from `refs/notes/commits`, but
 this default can be overridden.  See the OPTIONS, CONFIGURATION, and
@@ -39,9 +40,9 @@ message stored in the commit object, the notes are indented like the
 message, after an unindented line saying "Notes (<refname>):" (or
 "Notes:" for `refs/notes/commits`).
 
-Notes can also be added to patches prepared with `git format-patch` by
-using the `--notes` option. Such notes are added as a patch commentary
-after a three dash separator line.
+Notes contents can also be included in patches prepared with
+`git format-patch` by using the `--notes` option. Such notes are added
+as a patch commentary after a three dash separator line.
 
 To change which notes are shown by 'git log', see the
 "notes.displayRef" configuration in linkgit:git-log[1].
-- 
2.8.0.windows.1

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

> Done, I'll send a patch shortly. But I wanted to list the supported
> object types explicitly, in particular as many guide in the net are
> unclear that only annotated tags are object, and lightweight ones are
> not.

I'd really hate to see an explicit list of what object types there
are, as it is one more place we'd need to update if we ever add new
object types (and we are unlikely to do so anytime soon, which makes
it even more likely that we would forget there is this explicit list
we'd need to update).

-     Adds, removes, or reads notes attached to objects, without touching
-     the objects themselves.
+     Adds, removes, or reads notes attached to any object (not limited
+     to commit objects), without touching the objects themselves.

should be sufficient, no?

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

On Fri, Apr 1, 2016 at 5:31 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Sebastian Schuberth <sschuberth@gmail.com> writes:
>
>> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
>> ---
>>  Documentation/git-notes.txt | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
>> index 8de3499..5375d98 100644
>> --- a/Documentation/git-notes.txt
>> +++ b/Documentation/git-notes.txt
>> @@ -234,8 +234,9 @@ which operation triggered the update, and the commit authorship is
>>  determined according to the usual rules (see linkgit:git-commit[1]).
>>  These details may change in the future.
>>
>> -It is also permitted for a notes ref to point directly to a tree
>> -object, in which case the history of the notes can be read with
>> +It is also permitted for a notes ref to point to any other object in
>> +the object store besides commit objects, that is annotated tags, blobs
>> +or trees. For the latter, the history of the notes can be read with
>>  `git log -p -g <refname>`.
>
> I do not think this is correct place to patch.  The original is not
> talking about what objects can have notes attached at all.  What it
> explains is this.

Thanks for the explanation, I was indeed misreading this. I'll try to
clarify this section then, too. In order to do so, I think we should
mention how to actually create a <refname> that directly points to a
tree instead of a commit for the history of notes. Would you have an
example how to do that?

-- 
Sebastian Schuberth

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

>>> -It is also permitted for a notes ref to point directly to a tree
>>> -object, in which case the history of the notes can be read with
>>> +It is also permitted for a notes ref to point to any other object in
>>> +the object store besides commit objects, that is annotated tags, blobs
>>> +or trees. For the latter, the history of the notes can be read with
>>>  `git log -p -g <refname>`.
>>
>> I do not think this is correct place to patch.  The original is not
>> talking about what objects can have notes attached at all.  What it
>> explains is this.
>
> Thanks for the explanation, I was indeed misreading this. I'll try to
> clarify this section then, too. In order to do so, I think we should
> mention how to actually create a <refname> that directly points to a
> tree instead of a commit for the history of notes. Would you have an
> example how to do that?

Interesting.  This came from 9eb3f816 (Documentation/notes: document
format of notes trees, 2010-05-08):

    Documentation/notes: document format of notes trees

    Separate the specification of the notes format exposed in
    git-config.1 from the description of the option; or in other
    words, move the explanation for what to expect to find at
    refs/notes/commits from git-config.1 to git-notes.1.

    Suggested-by: Thomas Rast <trast@student.ethz.ch>
    Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

but I do not find a corresponding sentence that says a notes ref can
point at a tree in the text before the patch.

I highly suspect that "git notes add" and other Porcelain level
commands that manipulate an existing notes tree would be unhappy if
a notes ref is not a commit, as it is clear from the paragraph
before the one under discussion, i.e.

    Every notes change creates a new commit at the specified notes ref.
    You can therefore inspect the history of the notes by invoking, e.g.,
    `git log -p notes/commits`.  Currently the commit message only records
    which operation triggered the update, and the commit authorship is
    determined according to the usual rules (see linkgit:git-commit[1]).
    These details may change in the future.

that in order to create a "new" commit, setting the current one as
its parent, would require that the current one to be a commit and
not a bare tree.  "git notes list" and others that merely read from
the notes tree would probably work.