Fixed unavailable link in Documentation/howto/new-command.txt along… - #647
Fixed unavailable link in Documentation/howto/new-command.txt along…#647kyagi wants to merge 1 commit into
Documentation/howto/new-command.txt along…#647Conversation
|
Please make sure that your commit message focuses on answering the question "why?" more than on "how?", and that it wraps at <= 76 columns per line. It would be good to spend at least one paragraph on explaining the rationale, rather than a rather terse oneliner. Further, the first line of the commit message should be a short summary and use the imperative ("docs: fix the link to api-builtin" or something like that). Finally send the patch to the mailing list for review. You can use GitGitGadget, submitGit or send it manually. |
|
@kyagi gentle ping? |
|
@kyagi another gentle ping? |
|
I'm at a loss as to what to do... |
I tried to point out here what I think needs to be done... |
No. There are many Unicode characters out there which occupy more than one column. |
|
@kyagi so... does #647 (comment) not clarify what I think you will want to do? |
|
@kyagi is it really unclear what needs to be done? The first line of the commit message should look like this:
And the commit message should not focus on what the patch does, the diff is really eloquent enough about this. A much more interesting aspect that is totally unanswered in this entire PR, and that would be a fine question to answer in the commit message, is why this is a change you (and really, the Git developers) want. This needs to be answered. Currently, there is no answer to this question. If you need inspiration for good commit messages, just have a look at Git's commit history, it is full of excellent examples to follow. |
|
@kyagi gentle ping? |
|
Can I take over this PR? Lost half hour figuring where did the missing api-builtin.txt go! |
|
@abhishekkumar2718 you will have to ask @kyagi, not me... |
|
@kyagi Can I work on this or help you out with the patch?
Beginning with what Johannes said, ensure the commit message focuses on why? We need to make changes as described in this commit because documentation was moved from
First line of commit message is the title of commit and briefly explains the changes within. It should additionally be in the imperative mood since we are saying if applied, this commit will <title>. The sentence if applied, this commit will fixed unavailable link ... history does not make sense but if applied, this commit will fix the link to api-builtin makes perfect sense. So, overall your commit message might look like: Notice how I have wrapped at 72 columns. This helps people who use old-style 80 columns per line terminals be able to read the entire message. Nitpick for Johannes, it's better to wrap at 72 columns instead of 76 due to the right-hand margin. 1 |
It is better, but not required. If you care to look at my commit messages, you will see that I do use the 72 columns rule myself. As to going forward: if @kyagi does not reply, or does not want to update this PR, you could just send an alternative patch yourself. In that case, I would suggest linking to this here PR for reference. |
|
@abhishekkumar2718 It's my pleasure to have you work on this patch. Thank you. |
|
@abhishekkumar2718 Or you can send an alternative patch as Johannes said. It's up to you. |
e86e787 to
f9119fb
Compare
|
Let me hang on a minute. How about this commit message? |
It's longer than 50 characters. Maybe replace 'documentation' with 'doc'.
'... past changesets.' is a bit vague. It would be helpful if you were more specific like the commit hash or title. Reviewers save time since they don't have to refer back to your PR here to find out which changesets to be exact. |
|
I wonder if it needs to be specific. IMHO, commit hashes does not necessarily matter in this case because diff is convincing enough. Rather, a series of SHA-1 hashes of the old change sets might make it difficult for readers to understand the purpose of this change. @dscho What do you think? |
|
I think that @abhishekkumar2718's advice is pretty sound. |
f9119fb to
9536c65
Compare
|
@kyagi looks good. Wanna |
|
/submit |
|
Error: User kyagi is not permitted to use GitGitGadget |
|
Yes, please. |
|
/allow |
|
User kyagi is now allowed to use GitGitGadget. |
|
/submit |
|
Submitted as [email protected] |
|
On the Git mailing list, Jonathan Nieder wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
There is an issue in commit e18d57845e6745fc4a31b4e03fb0929f7fba02e6: |
92520b8 to
51a67b9
Compare
ec14d4e had moved documentation from api-builtin.txt to builtin.h. This patch updates new-command.txt to reflect that change. The following describes how the documentation has finally been placed in this file, over the related changesets. +-----------------+ *OLD LINK* +-----------------+ | api-builtin.txt | <~~~~~~~~~~ | api-command.txt | +-----------------+ +-----------------+ | ~ * | | deleted, ~ N | moved and renamed from | contents is taken over ~ E | Documentation/technical/ | by builtin.h ~ W | to | (this file) ~ | Documentation/howto/ | ~ L | | ~ I | v ~ N v +-----------+ ~ K +-----------------+ | builtin.h | <~~~~~~~~~~~~ * | new-command.txt | +-----------+ +-----------------+ ---> moved to(or renamed to) ~~~> refers to Signed-off-by: Kazuo Yagi <[email protected]>
51a67b9 to
0a863e6
Compare
… with the changeset history.
Signed-off-by: Kazuo Yagi [email protected]
Thanks for taking the time to contribute to Git! Please be advised that the
Git community does not use github.com for their contributions. Instead, we use
a mailing list ([email protected]) for code submissions, code reviews, and
bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
to conveniently send your Pull Requests commits to our mailing list.
Please read the "guidelines for contributing" linked above!