Re: [PATCH 1/1] Extract only the message body from git commit.
"Hardik Kumar" <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
> nit: I think this can be written more clearly. Let's use present tense > and state things affirmatively: Sure, I will update this in a later commit. > Reading the code, this doesn't seem to do what the log says it does. > Testing it against what it did before this patch: > We can see that the previous output only shows the commit log, title > + body. There were no SHAs, tree, etc., the opposite of what this > patch's log claimed. I see, I was comparing this to the how git stores your commits in .git/objects. My assumption was the function would take an object from there and (wanting to extract just the body) was taking the entire object as is and not stripping out the subject. > What this patch actually does is drop the commit subject. Yes, since I suppose all the other places in the file where this method is being used don't require the subject line. > Either way, this patch does not address the '# fixme' correctly. > > Before continuing, I think we should try to understand what the '# fixme' > meant in the first place. My assumption was that the function that I made changes to was solely responsible for sending back only the message stripping out subject and metadata related to the commit object. Moving forward I suppose the one suggestion would be changing the name of the variable `foundTitle` to `foundSubject` instead since that might result in less ambuiguity in what the function is doing (and the fixme). Although I dont't think if that really makes any difference. But yes the #fixme does seem very vague now. Not sure if this is worth it. Regards, Hardik.