tclint, pre-commit and patch submissions

tclint, pre-commit and patch submissions

[Tom de Vries via Gdb]

Hi,

I've recently added a tclint pre-commit hook, and started cleaning the 
testsuite.

The current status is that all gdb.* dirs are done, with the exception 
of gdb.stabs (which is going to be removed, so I skipped it).

My question is what to do with patches adding new test-cases or 
modifying existing test-cases.

If these patches are not tclint-clean, then after commit this command:
...
$ pre-commit run tclint --all-files
...
will start showing new tclint errors.

One thing that could be done at that point is to ask the 
submitter/committer to fix the tclint errors.

But there's no formal agreement atm that this need to be fixed.  I've 
proposed the hook, and Tom Tromey approved it, but that's just two 
maintainers.

So I'd like to know the opinion of other maintainers.

Possible outcomes of this discussion could be that:
- we get rid of tclint in pre-commit and forget about it
- we drop one or more error categories in gdb/tclint.toml
- we require submitters to run pre-commit before submission
- we make the repo refuse commits that are not pre-commit clean
- nothing changes, and submitters can use all/some/no pre-commit hooks
   for their own submissions

Thanks,
- Tom

Re: tclint, pre-commit and patch submissions

[Guinevere Larsen via Gdb]

On 10/1/25 8:55 AM, Tom de Vries wrote:
> Hi,
>
> I've recently added a tclint pre-commit hook, and started cleaning the 
> testsuite.
>
> The current status is that all gdb.* dirs are done, with the exception 
> of gdb.stabs (which is going to be removed, so I skipped it).
>
> My question is what to do with patches adding new test-cases or 
> modifying existing test-cases.
>
> If these patches are not tclint-clean, then after commit this command:
> ...
> $ pre-commit run tclint --all-files
> ...
> will start showing new tclint errors.
>
> One thing that could be done at that point is to ask the 
> submitter/committer to fix the tclint errors.
>
> But there's no formal agreement atm that this need to be fixed. I've 
> proposed the hook, and Tom Tromey approved it, but that's just two 
> maintainers.

I think using a linter is good, and making new submissions clean for 
that linter is something worthwhile.

IMO, when approving a patch, we could just mention the TCL lint output 
as nits to be fixed before a patch is pushed.

>
> So I'd like to know the opinion of other maintainers.
>
> Possible outcomes of this discussion could be that:
> - we get rid of tclint in pre-commit and forget about it
> - we drop one or more error categories in gdb/tclint.toml
> - we require submitters to run pre-commit before submission
> - we make the repo refuse commits that are not pre-commit clean
> - nothing changes, and submitters can use all/some/no pre-commit hooks
>   for their own submissions
>
> Thanks,
> - Tom
>

-- 
Cheers,
Guinevere Larsen
It/she

Re: tclint, pre-commit and patch submissions

[Luis via Gdb]

Hi Tom,

I'm in favor o enforcing clean commits. If we are using a linter for TCL,
then I'd personally go with this option:

- we make the repo refuse commits that are not pre-commit clean

Or a pre-commit checker that we carry on our repo that gets run
automatically.


On Wed, Oct 1, 2025, 12:55 Tom de Vries <tdevries@suse.de> wrote:

> Hi,
>
> I've recently added a tclint pre-commit hook, and started cleaning the
> testsuite.
>
> The current status is that all gdb.* dirs are done, with the exception
> of gdb.stabs (which is going to be removed, so I skipped it).
>
> My question is what to do with patches adding new test-cases or
> modifying existing test-cases.
>
> If these patches are not tclint-clean, then after commit this command:
> ...
> $ pre-commit run tclint --all-files
> ...
> will start showing new tclint errors.
>
> One thing that could be done at that point is to ask the
> submitter/committer to fix the tclint errors.
>
> But there's no formal agreement atm that this need to be fixed.  I've
> proposed the hook, and Tom Tromey approved it, but that's just two
> maintainers.
>
> So I'd like to know the opinion of other maintainers.
>
> Possible outcomes of this discussion could be that:
> - we get rid of tclint in pre-commit and forget about it
> - we drop one or more error categories in gdb/tclint.toml
> - we require submitters to run pre-commit before submission
> - we make the repo refuse commits that are not pre-commit clean
> - nothing changes, and submitters can use all/some/no pre-commit hooks
>    for their own submissions
>
> Thanks,
> - Tom
>

Re: tclint, pre-commit and patch submissions

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> If these patches are not tclint-clean, then after commit this command:
Tom> ...
Tom> $ pre-commit run tclint --all-files
Tom> ...
Tom> will start showing new tclint errors.

Tom> - we make the repo refuse commits that are not pre-commit clean

This one is tricky because we'd have to manage this stuff on the server
side.

Tom> - nothing changes, and submitters can use all/some/no pre-commit hooks
Tom>   for their own submissions

So far this hasn't been that big an issue IMO.  I think most of the
regular contributors use pre-commit.

Tom