Adding checks for non-ASCII filenames (was: [Gerrit] gitconfig: Add sample checks shipped by git to pre-commit hook)

[Paul Menzel]

Dear coreboot folks,

Am Donnerstag, den 30.05.2013, 01:57 +0200 schrieb gerrit code review:

From http://review.coreboot.org/3336:
> Change subject: gitconfig: Add sample checks shipped by git to pre-commit hook
> ......................................................................
>
> Patch Set 2: Code-Review-2
>
> It does not make sense to have a hook that checks a tree-wide policy
> and then allow that policy to be disabled by configuration. Either we
> decide on the filename policy or we do not need the hook.

as non-ASCII characters are not allowed in our source [1], I assume they
are also not allowed in file names.

The sample pre-commit hook shipped by git has a check for non-ASCII file
names, which can be disabled by a config option.

Are there any circumstances where non-ASCII characters might be needed?
If not, Peter is right and the option can be removed from the script,
which I would do then.

Thanks,

Paul

[1] http://review.coreboot.org/2604

Adding checks for non-ASCII filenames (was: [Gerrit] gitconfig: Add sample checks shipped by git to pre-commit hook)

[Peter Stuge]

Paul Menzel wrote:
> as non-ASCII characters are not allowed in our source [1]

Where the hell did you get the idea that there is such a policy?

You are extrapolating discussion about four characters in two files
between two individuals to a tree-wide project-wide policy. That is
absolutely unreasonable!
> I assume they are also not allowed in file names.

Don't assume.. Especially not based on extrapolation like that.
> The sample pre-commit hook shipped by git has a check for non-ASCII
> file names, which can be disabled by a config option.

Either we have a policy or we do not. If we have a policy then why do
we care what the sample hook does?
> Are there any circumstances where non-ASCII characters might be needed?

I don't think so and quite likely they would break things, so I
would be in favor of a policy to only allow ASCII filenames.
> If not, Peter is right and the option can be removed from the
> script, which I would do then.

You shouldn't have pushed a commit in the first place without
strong consensus to back it up. Working together, communication,
all that.

//Peter

[RFC] Add policy and check for non-ASCII file names

[Paul Menzel]

Am Donnerstag, den 30.05.2013, 12:10 +0200 schrieb Peter Stuge:
> Paul Menzel wrote:
> > as non-ASCII characters are not allowed in our source [1]
>
> Where the hell did you get the idea that there is such a policy?

Because one of the project leaders reverted it and reading Ron’s commit
message of the revert.
> You are extrapolating discussion about four characters in two files
> between two individuals to a tree-wide project-wide policy. That is
> absolutely unreasonable!

Sorry.
> > I assume they are also not allowed in file names.
>
> Don't assume.. Especially not based on extrapolation like that.
> > The sample pre-commit hook shipped by git has a check for non-ASCII
> > file names, which can be disabled by a config option.
>
> Either we have a policy or we do not. If we have a policy then why do
> we care what the sample hook does?
> > Are there any circumstances where non-ASCII characters might be needed?
>
> I don't think so and quite likely they would break things, so I
> would be in favor of a policy to only allow ASCII filenames.

Alright. Good to know.

Does somebody object to such a policy?
> > If not, Peter is right and the option can be removed from the
> > script, which I would do then.
>
> You shouldn't have pushed a commit in the first place without
> strong consensus to back it up. Working together, communication,
> all that.

Sorry. I hope I fixed that by sending this message to the list. In
pre-Gerrit times I would have tagged the subject with [RFC] (request for
comments). And as patches can be discussed in Gerrit, I do not see why
pushing it to Gerrit is a problem.

Thanks,

Paul

[RFC] Add policy and check for non-ASCII file names

[Peter Stuge]

Paul Menzel wrote:
> > > as non-ASCII characters are not allowed in our source [1]
> >
> > Where the hell did you get the idea that there is such a policy?
>
> Because one of the project leaders reverted it and reading Ron’s
> commit message of the revert.

That is still only one person. It's a good idea to take more input.
> > You shouldn't have pushed a commit in the first place without
> > strong consensus to back it up. Working together, communication,
> > all that.
>
> Sorry. I hope I fixed that by sending this message to the list.

Yes, and thanks for taking it to the list. It's the right thing to do.
> And as patches can be discussed in Gerrit, I do not see why pushing
> it to Gerrit is a problem.

Gerrit is a code review tool, not a communications system.

Email is a communications system, not a code review tool.

Code review requires careful attention to detail, communication
perhaps slightly less so since there's a human on the other end.

Gerrit is a bad message board. Use the mailing list for discussion.

Thanks

//Peter