Item11808: perltidy check on checkin

Extend the checkin trigger to check if the file being checked in has been perltidied.

Do this by running perltidy -b on the input file and diffing with the original input. If there are any differences, the file has not been perltidied.

-- CrawfordCurrie - 01 May 2012

Does that mean EVERYTHING is going to be perltidied on check in?

I don't want this for non-core plugins!!!

The recent checkins broke a couple of plugins as well.

Please respect CoordinateWithAuthor.

Please revert the automatic post-checkin handler that perltidies all code unasked immediately!

-- MichaelDaum - 07 May 2012

The trigger is a check. It does not permit checkins of anything that is not tidy; however it does not tidy before checkin. Babar decided to tidy everything and check it in, as a service to authors I guess.

Everything under tools should be tidy as well. However tidying under lib/CPAN is wrong, I agree, because these modules are imported from CPAN.

-- CrawfordCurrie - 07 May 2012

Micha, please calm down. You're the only one who persists in coding in your own way, even though I've told you many times it's even impacting your own business (for example, I refuse to debug any plugin that's coded with your tidyness, as I can't read it fast enough, and I don't want to spend unnecessary time decoding it).

Anyway, I agree that perltidy'ing the lib/CPAN/lib wasn't smart, and I've reverted it. As for the CoordinateWithAuthor, I disagree with your conclusion. It's not changing the code, nor even the license, it was barely changing the formatting so it's uniform across the entire repository.

You refuse it vehemently, fair enough. I've now coded the pre-commit.pl so that it will not do any check on any plugin which is CoordinateWithAuthor. I still think it's not a good criteria, but I don't want to argue about something everybody (but Micha) has been asking for for years now.

I think the proper way would be to allow plugin maintainers to supply their own perltidy configuration, and ensure that their own plugins are tidied accordingly.

Thanks Crawford for doing the initial commit.

I think I've reverted everything that shouldn't have been committed in trunk. I'll revert the CPAN libs in the release branch shortly.`

-- OlivierRaginel - 07 May 2012

OK, everything which shouldn't have been committed has been reverted, and the pre-commit.pl hook now checks CoordinateWithAuthorToJSON (updated daily directly by the hook) to know whether it should enforce the tidyness or not. Plugin authors who want to enforce tidyness, but keep CoordinateWithAuthor are free to modify the SEARCH on this page to remove their plugin.

Not sure it's such a good idea, as if the page fails, then the pre-commit will fail, therefore nobody will be able to checkin...

We'll see if we have to restrict access.

Closing, as everything that I know of has been implemented.

-- OlivierRaginel - 07 May 2012

Reopening again as there are still things wrong again.

I am not aware of anybody asking to perltidy the complete code base, nor that we desperately need a kind of perltidy regime beyond what we have in the core and associated plugins.

My code not being readable: that's plain bullshit.

• I said I can't read your code. You can't read mine. Fair enough. -- OlivierRaginel

We once had a discussion how to format perl best and we weren't able to find a consensus. That's why we decided to use perltidy without any customization for the core and assoc plugins. We also agreed those days that plugin authors are free to use a perltidiness they chose.

Again: we never agreed to expand the code formating guidelines of the core to all of the code base. Never, ever. Prove me wrong.

For the records, here's my perltidyrc

-q 
-l=0 
-i=2 
-pt=2 
-nsfs 
-ce
-novalign

that results in much better readability for my taste.

-- MichaelDaum - 07 May 2012

Recent changes reverted my work on at least CaptchaPlugin, StringifierContrib. Now I fail to check in the restored work. This is insane. Please remove whatever blocks me checking in code.

No, I can't calm down on this one

-- MichaelDaum - 07 May 2012

Yes you can, especially since I restored your work which I wrongly erased, and I already apologized for it. Was a stupid mistake on my side.

Apart from that, I've asked you about what was best for you to implement so you could work properly, and we could have a global perltidy checker. You told me to respect CoordinateWithAuthor, which I did.

Now I offer again what I offered before:

Would it be ok to implement a perltidyrc, which when set at the top level of a plugin, would be used to test for perltidyness? Also accepting an OFF flag?

So, to give some examples, you commit a file called StringifierContrib/perltidyrc, which contains what you posted above, and the pre-commit.pl hook would use it for all code within StringifierContrib.

Or you commit a CaptchaContrib/perltidyrc which contains just OFF, and no perltidy checking will be done for all code within CaptchaContrib.

This would prevail over CoordinateWithAuthor, which we would still keep in place.

Problem comes for plugin which do not have a topic on foswiki.org, like BibtexPlugin, or CaptchaPlugin. CoordinateWithAuthor cannot apply, so we have to resort to the perltidyrc.

-- OlivierRaginel - 07 May 2012

Further discussion going on at CodingStandardsDiscussions

-- MichaelDaum - 08 May 2012

perltidy v20120714 enforced a few more rules, typically removing final new lines, and as I upgraded foswiki.org, everybody has to upgrade or risk having issues. To ease the process, I've once again perltidy'ed everything BUT the CoordinateWithAuthors. I'll fine-tune it tomorrow.

-- OlivierRaginel - 26 Sep 2012

This is a little unsettling and terribly inconvenient for casual developers... to sweeten things I've updated documentation, see Development.TIDY and linked to it in a newly updated (hopefully more helpful) commit reject message.

-- PaulHarvey - 26 Sep 2012

Re-opening to force the release branch to be tidy'ed too.

-- OlivierRaginel - 25 Oct 2012

Sorry George, I forgot to close that one!

-- OlivierRaginel - 02 Dec 2012