Re: [PATCH v2 1/1] config: move documentation to config.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 23, 2019 at 9:59 AM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
>
> On Tue, Oct 22, 2019 at 07:05:06AM +0000, Heba Waly via GitGitGadget wrote:
> > From: Heba Waly <heba.waly@xxxxxxxxx>
> >
> > Move the documentation from Documentation/technical/api-config.txt into
> > config.h
>
> This is still a little thin for what we usually want from commit
> messages. Try to imagine that five years from now, you find this commit
> by running `git blame` on config.h and then examining the commit which
> introduced all these comments with `git show <commit-id>` - what would
> you want to know?
>
> Typically we want to know "why" the change was made, because the diff
> shows "what". We can see from the diff that you're moving comments from
> A to B, but if you explain why you did so (not "because my Outreachy
> mentor told me to" ;) but "because it is useful to see usage information
> next to code" or "this is best practice as described by blah blah") - I
> wouldn't be able to know that reasoning just from looking at your diff.
Ok, got it.

>
> > diff --git a/config.h b/config.h
> > index f0ed464004..02f78ffc2b 100644
> > --- a/config.h
> > +++ b/config.h
> > @@ -4,6 +4,23 @@
> >  #include "hashmap.h"
> >  #include "string-list.h"
> >
> > +
> > +/**
> > + * The config API gives callers a way to access Git configuration files
> > + * (and files which have the same syntax). See linkgit:git-config[1] for a
>
> Ah, here's another place where the Asciidoc link isn't going to do
> anything anymore.
yep!
> Otherwise I didn't still see anything jumping out. When the commit
> message is cleaned up I'm ready to add my Reviewed-by line.
great!

>  - Emily

Thanks :)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux