[QGIS-Developer] pre-commit for code-formatting
Julien Cabieces
julien.cabieces at oslandia.com
Thu Dec 12 07:33:50 PST 2024
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.
> here you go https://github.com/qgis/QGIS/pull/59857
>
> Le jeu. 12 déc. 2024 à 15:12, Jean Felder via QGIS-Developer <qgis-developer at lists.osgeo.org> a écrit :
>
> Le 12/12/2024 à 14:42, Denis Rouzaud via QGIS-Developer a écrit :
>
> Le jeu. 12 déc. 2024 à 14:22, Julien Cabieces <julien.cabieces at oslandia.com> a écrit :
>
> Oh, yes, I was looking at an old version of the documentation.
>
> > I think it's a great tool on developer side but I have my concerns about
> > the CI part. It adds a lot of automatic commit which doesn't help when
> > you want to read the git history. If you want to blame a specific line
> > of code, you can land in this type of commit instead of the relevant
> > modification you were heading to. You can trace it back to the pull
> > request but it's one more step, and it makes us even more attached to the
> > GitHub platform.
> >
> > Thanks to Nyall's hint, I have added the refactoring commits to
> https://github.com/qgis/QGIS/blob/master/.git-blame-ignore-revs
> > When I blame files online in Github it seems to be working:
> > For instance, https://github.com/qgis/QGIS/blame/master/src/gui/actions/qgsactionmenu.h
> > Last commit is the pre-commit run but you don't see it in the blame.
> >
>
> This works for the refactoring commits but not for all the pre-commit
> commits added in Pull Requests.
>
> I see only 2 options there:
> 1. We do not auto update PR (basically same behavior than before) and ask people to setup the pre-commit hook
> 2. We automatically add the commits to the .git-blame-ignore-revs (possibly after merge to avoid merging conflicts !!!) (might be
> an issue if it grows too big)
>
> Thoughts?
>
> Denis
>
> Hi,
>
> First of all, Thank you Denis for working on this.
>
> I guess that most of the regular contributors will activate the pre-commit hook at some point.
> However, this probably won't happen for first-time or casual contributors and having a red CI because of some formatting error can
> be quite frustrating. So, I would favor option 2.
>
> Based, on this discussion
> (https://stackoverflow.com/questions/76367362/what-are-the-performance-characteristics-of-git-blame-ignore-revs), it looks like
> that the size `.git-blame-ignore-revs` does not matter.
>
> 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
--
Julien Cabieces
Senior Developer at Oslandia
julien.cabieces at oslandia.com
More information about the QGIS-Developer
mailing list