Skip to content

Conversation

algonell
Copy link

@algonell algonell commented Sep 17, 2024

Fix typos in documentation, comments, etc.

cc: Eric Sunshine [email protected]
cc: "Kristoffer Haugsbakk" [email protected]

@algonell algonell marked this pull request as ready for review September 18, 2024 08:31
@dscho
Copy link
Member

dscho commented Sep 18, 2024

@algonell how would you feel about force-pushing and keeping one PR open for these typo fixes?

@algonell
Copy link
Author

@dscho absolutely, I'm a bit worried about the OS X failing / stuck jobs...

@dscho
Copy link
Member

dscho commented Sep 18, 2024

I'm a bit worried about the OS X failing / stuck jobs...

Right, that's something nobody seems to take care of.

On the other hand, since your typo fixes do not touch code, you can ignore failing CI builds, right?

@algonell
Copy link
Author

algonell commented Sep 18, 2024

Well yes, but one of my uptodate -> up to date changes in unpack-trees.c requires the according t7110-reset-merge.sh test to be updated as well (did not commit this one yet).

That's why I'm extra cautious about it.

So should I submit?

@dscho
Copy link
Member

dscho commented Sep 18, 2024

one of my uptodate -> up to date changes in unpack-trees.c requires the according t7110-reset-merge.sh test to be updated as well (did not commit this one yet).

That's why I'm extra cautious about it.

So should I submit?

Does this change a user-visible message? If so, it might be fraught with peril to change it: Third-party applications do not enjoy any consistent error types being reported by Git, so they have to parse the error messages. And if those error messages change, those applications break.

@algonell
Copy link
Author

Agreed, a potential breaking change, that's why I skipped it.

@algonell
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 19, 2024

Error: Ignoring PR with empty title and/or body

@algonell
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 19, 2024

Error: Could not determine full name of algonell

@algonell
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 19, 2024

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1794/algonell/fix-typos-v1

To fetch this version to local tag pr-1794/algonell/fix-typos-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1794/algonell/fix-typos-v1

Copy link

gitgitgadget bot commented Sep 19, 2024

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Thu, Sep 19, 2024 at 2:35 PM Andrew Kreimer via GitGitGadget
<[email protected]> wrote:
> Fix typos in documentation, comments, etc.
>
> Andrew Kreimer (20):

Thanks. These all look fine.

Out of curiosity, did you use a tool to discover these mistakes?

Copy link

gitgitgadget bot commented Sep 19, 2024

User Eric Sunshine <[email protected]> has been added to the cc: list.

@algonell
Copy link
Author

Thank you Eric!

Sure, my secret sauce is codespell.

Copy link

gitgitgadget bot commented Sep 19, 2024

On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this):

All of these look correct.

Maybe all of the commits could be combined into one commit with the
message “treewide: fix typos”.  But that wasn’t asked for last time.[1]
So I dunno.

🔗 1: https://lore.kernel.org/git/[email protected]/

-- 
Kristoffer Haugsbakk

Copy link

gitgitgadget bot commented Sep 19, 2024

User "Kristoffer Haugsbakk" <[email protected]> has been added to the cc: list.

Copy link

gitgitgadget bot commented Sep 19, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Kristoffer Haugsbakk" <[email protected]> writes:

> All of these look correct.
>
> Maybe all of the commits could be combined into one commit with the
> message “treewide: fix typos”.  But that wasn’t asked for last time.[1]
> So I dunno.

It depends.  When done carefully to make sure that nothing textually
conflicts with any topics in flight, a single "treewide" patch is
slightly nicer.  If nothing conflicts with nothing else, either
format is fine.  If something have nasty conflicts (e.g., a refactor
moved the comment with typos far from the original location or even
to a different file), individual patch form is easier to discard the
ones that need to wait.

I do not know which case this 20-patch collection falls into.

Thanks.

Copy link

gitgitgadget bot commented Sep 19, 2024

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Thu, Sep 19, 2024 at 4:42 PM Junio C Hamano <[email protected]> wrote:
> "Kristoffer Haugsbakk" <[email protected]> writes:
> > Maybe all of the commits could be combined into one commit with the
> > message “treewide: fix typos”.  But that wasn’t asked for last time.[1]
> > So I dunno.
>
> It depends.  When done carefully to make sure that nothing textually
> conflicts with any topics in flight, a single "treewide" patch is
> slightly nicer.  If nothing conflicts with nothing else, either
> format is fine.  If something have nasty conflicts (e.g., a refactor
> moved the comment with typos far from the original location or even
> to a different file), individual patch form is easier to discard the
> ones that need to wait.
>
> I do not know which case this 20-patch collection falls into.

For what it's worth, I found the submission easier to review as
separate patches because it allowed me to review a small batch, do
something else for a bit, review another batch, do something else,
etc., without losing my place since I deleted the ones I had already
reviewed, so I knew that those remaining in my Inbox were still
pending review.

Copy link

gitgitgadget bot commented Sep 19, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Eric Sunshine <[email protected]> writes:

> For what it's worth, I found the submission easier to review as
> separate patches because it allowed me to review a small batch, do
> something else for a bit, review another batch, do something else,
> etc., without losing my place since I deleted the ones I had already
> reviewed, so I knew that those remaining in my Inbox were still
> pending review.

It is a very good point.

FWIW, all except for one apply cleanly to both v2.46.0 and 'seen',
and one fixes a typo introduced between v2.46.0 and 'master'.

Will queue.
Thanks.

@dscho
Copy link
Member

dscho commented Sep 19, 2024

Thank you Eric!

Sure, my secret sauce is codespell.

@algonell please reply on-list.

Copy link

gitgitgadget bot commented Sep 19, 2024

This patch series was integrated into seen via git@c9a92b0.

Copy link

gitgitgadget bot commented Sep 19, 2024

This patch series was integrated into next via git@07777d1.

Copy link

gitgitgadget bot commented Sep 20, 2024

On the Git mailing list, Andrew Kreimer wrote (reply to this):

On Thu, Sep 19, 2024 at 03:35:25PM -0400, Eric Sunshine wrote:
> On Thu, Sep 19, 2024 at 2:35 PM Andrew Kreimer via GitGitGadget
> <[email protected]> wrote:
> > Fix typos in documentation, comments, etc.
> >
> > Andrew Kreimer (20):
> 
> Thanks. These all look fine.
> 
> Out of curiosity, did you use a tool to discover these mistakes?

Thank you all!

Sure, my secret sauce is codespell:
https://github.com/codespell-project/codespell

@algonell algonell closed this Sep 20, 2024
@dscho
Copy link
Member

dscho commented Sep 20, 2024

@algonell did you really mean to close this PR? If you plan on submitting a v2, GitGitGadget is set up to do that only when /submiting from the same PR, not from a new one.

@algonell
Copy link
Author

@dscho apologies for the extra noise...

Actually I wanted to sync and rebase some new things (from my next branch)

I think I better to send patches via git send-email from now on (already started).

Thinking out loud: the bot should not close an 'empty' PR...?!

@dscho
Copy link
Member

dscho commented Sep 20, 2024

Thinking out loud: the bot should not close an 'empty' PR...?!

That's not the bot, that's GitHub doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants