| From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: run pgindent on a regular basis / scripted manner |
| Date: | 2023-01-22 15:01:13 |
| Message-ID: | 55588553-a433-96ec-fdfe-fd46ac6f3a4a@dunslane.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2023-01-21 Sa 11:10, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>>> I think we could do better with some automation tooling for committers
>>>> here. One low-risk and simple change would be to provide a
>>>> non-destructive mode for pgindent that would show you the changes if any
>>>> it would make. That could be worked into a git pre-commit hook that
>>>> committers could deploy. I can testify to the usefulness of such hooks -
>>>> I have one that while not perfect has saved me on at least two occasions
>>>> from forgetting to bump the catalog version.
> That sounds like a good idea from here. I do not think we want a
> mandatory commit filter, but if individual committers care to work
> this into their process in some optional way, great! I can think
> of ways I'd use it during patch development, too.
Yes, it's intended for use at committers' discretion. We have no way of
forcing use of a git hook on committers, although we could reject pushes
that offend against certain rules. For the reasons you give below that's
not a good idea. A pre-commit hook can be avoided by using `git commit
-n` and there's are similar option/hook for `git merge`.
>
> (One reason not to want a mandatory filter is that you might wish
> to apply pgindent as a separate commit, so that you can then
> put that commit into .git-blame-ignore-revs. This could be handy
> for example when a patch needs to change the nesting level of a lot
> of pre-existing code, without making changes in it otherwise.)
Agreed.
> Looks reasonable, but you should also update
> src/tools/pgindent/pgindent.man, which AFAICT is our only
> documentation for pgindent switches. (Is it time for a
> --help option in pgindent?)
>
>
Yes, see revised patch.
> ... btw, can we get away with making the diff run be "diff -upd"
> not just "diff -u"? I find diff output for C files noticeably
> more useful with those options, but I'm unsure about their
> portability.
I think they are available on Linux, MacOS and FBSD, and on Windows (if
anyone's actually using it for this) it's likely to be Gnu diff. So I
think that's probably enough coverage.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| pgindent_silent_plus_showdiff-3.patch | text/x-patch | 4.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dean Rasheed | 2023-01-22 15:06:50 | Re: [PATCH] Use 128-bit math to accelerate numeric division, when 8 < divisor digits <= 16 |
| Previous Message | Karl O. Pinc | 2023-01-22 14:09:03 | Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences |