Item12806: pre-commit should check validity of TOPICINFO

When checking in shipped topics, there should be a check for TOPICINFO:

• user must be ProjectContributor
• date must be within 3 days of current date
• version must be 1
• other fields must be left alone

-- CrawfordCurrie - 19 Mar 2014

Attachments should be addressed to ProjectContributors as well.

-- MichaelDaum - 19 Mar 2014

Sorry, had to revert your changes to pre-commit.pl. Somehow Foswiki::Attrs is not found on f.o. which leads to an exit 1 for pre-commit and therefore prevent all further checkins.

http://irclogs.foswiki.org/bin/irclogger_log/foswiki?date=2014-03-19,Wed&sel=74#l70

-- OliverKrueger - 19 Mar 2014

Crawford. I'm not sure about svn, but for the git exit, adding the following to the .pl script seems to resolve the issue:

BEGIN {
    unshift @INC, './core/lib';
}

I've not gone any further than a quick test, but I'll add this to the pre-commit git client hook, so the file is updated prior to the push or dcommit.

-- GeorgeClark - 20 Mar 2014

Sorry, my fault - I had to rush out unexpectedly before I could properly test.

-- CrawfordCurrie - 20 Mar 2014

Re-opening because MichaelDaum is right, it ought to check attachments.

-- CrawfordCurrie - 20 Mar 2014

We may need to rethink this one a bit when moving to Git. The time between the commit and the push to the repo might be often longer than 3 days, especially if we put in place a process to accept pull requests from non-core contributors.

I've been converting our server-side svn hooks into a client side git hook that performs the same checks at the local commit time.

-- GeorgeClark - 20 Mar 2014

All the check is for is to make sure the date is sane. Another approach would be to compare against the date at the last checkin - it clearly can't be the same.

Anyway, this task is about the SVN hooks, so closing it for now.

-- CrawfordCurrie - 20 Mar 2014

Crawford, All the issues I had reverting my TinyMCE changes notwithstanding, I'm beginning to think that this is rather draconian, and off-putting for the occasional dev.

This is especially true if we ever want to permit edits of wiki topics from within the wiki. Instead of editing a topic in the wiki and then just checking it in, the user to then edit the file on disk, calculate correct unix epoch timestamps, and fix the TOPICINFO and attachment META.

We already scrub the TOPICINFO in BuildContrib, so none of this can make it into a release anyway, so I think we're just adding roadblocks to the new developer.

GuilainC was just asking earlier today how he could go about contributing some documentation changes. We don't currently allow edit on foswiki.org, so he has to run from a checkout. But the more I thought through how to describe the process, and then how to fix up the topics so the commits are not rejected. It begins to seem insurmountable.

Ideally,

• Ideally: DocumentationGroup could just edit on f.o, and request a commit from there. Sven started trying to implement this, but things stalled.
• Or maybe: User runs from svn or git, Edits in the local wiki, and checks in the changes. Maybe we could provide a utility to make that easier.

So... if this were a FeatureProposal, I guess I'd raise an objection I think the RM is the only one who actually benefits from this anyway, as that's who's stuck with the cleanup.

Oh...and if you enforce this for CompareRevisionsAddOnDemoTopic.txt you'll break the release. BuildContrib has an exception for that topic.

-- GeorgeClark - 25 Mar 2014

I've got another question. Is it your intention to enforce this for all topics in the repo, or just when "checking in shipped topics" ? It probably ought to only enforce the topic we ship in a release. IIRC the TestCase web has crafted TOPICINFO in places. And contributed extensions are a bit of a jungle.

-- GeorgeClark - 25 Mar 2014

W.R.T editing from the wiki - I got bored waiting for someone to implement that, and decided it was more important not to risk system slowdown due to missing/diseased TOPICINFO. Any automatic checkin of wiki edits can trivially implement this anyway.

Your point about BuildContrib is fair enough, except that the t.f.o test system doesn't use BuildContrib - it's a pseudo-install. So missing TOPICINFO banjaxes it, and means were are not testing what we ship (or something close to it).

Y, I could have gone through the feature proposal process; except that process is intended for end user features, not dev system features, and would have taken too long.

It uses the existence of the TIDY file, same as for source code.

-- CrawfordCurrie - 26 Mar 2014

Commenting is disabled while not logged in