[QGIS-Developer] pre-commit for code-formatting

Stefanos Natsis uclaros at gmail.com
Thu Dec 12 13:22:25 PST 2024


I also don't like the commit bot.
I've already had to force push, which I don't like to as it usually
disturbs reviewing, because it committed a change even though pre-commit
was ran locally!
https://github.com/qgis/QGIS/pull/59790/commits/4fc5d7011b8dbb234850f7147b69825d275f6f66

Stefanos

On Thu, 12 Dec 2024 at 21:41, Denis Rouzaud via QGIS-Developer <
qgis-developer at lists.osgeo.org> wrote:

> I would argue in favor of the auto update:
>
> * it's easier for newcomers: having a pull request rejected because of
> indentation is not welcoming
> * it's easier for online editing
> * the argument that you need to rebase is not valid to me:
>     * you can always force push and the code beautifier will be run on top
> again
>     * the solution you mention is to have an environment properly set up:
> if you do so, you won't have pre-commit commits in the log
>
>
>
>
> Le jeu. 12 déc. 2024 à 16:53, Jean Felder via QGIS-Developer <
> qgis-developer at lists.osgeo.org> a écrit :
>
>> Le 12/12/2024 à 16:38, Even Rouault via QGIS-Developer a écrit :
>> >
>> >
>> > Le 12/12/2024 à 16:33, Julien Cabieces via QGIS-Developer a écrit :
>> >> Not a great fan of this approach, It looks like complicated to me, I
>> would prefer the old behavior.
>> >>
>> >> Those automatic PR force also contributor to rebase before pushing new
>> >> modification to their branch. And if they have modified area of code
>> >> which have been reformatted, they get conflicts.
>> >>
>> >> automatic pre-commit commits brings more difficulties than what it
>> >> solves IMHO.
>> >
>> > I also tend to agree on that. On other projects I'm involved too
>> > (GDAL, PROJ, shapelib, libtiff) where we have put in place pre-commit,
>> > we just have a CI check that checks the formatting is OK. This
>> > requires a bit of education to first time contributors, but pointing
>> > to the documentation with clear instructions on how to setup
>> > pre-commit should be good enough.
>> >
>> https://github.com/qgis/QGIS/edit/master/.github/PULL_REQUEST_TEMPLATE.md
>> > could also have some words about it
>> >
>> > Like in
>> >
>> https://github.com/OSGeo/gdal/edit/master/.github/PULL_REQUEST_TEMPLATE.md
>> > , " - [ ] Make sure code is correctly formatted (cf [pre-commit
>> > configuration](
>> https://gdal.org/development/dev_practices.html#commit-hooks))"
>> >
>> On second thoughts, I agree with Julien and Even.
>>
>> Jean
>>
>>
>> _______________________________________________
>> QGIS-Developer mailing list
>> QGIS-Developer at lists.osgeo.org
>> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>>
> _______________________________________________
> QGIS-Developer mailing list
> QGIS-Developer at lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20241212/81287cfa/attachment.htm>


More information about the QGIS-Developer mailing list