Commit Checks #818

softworkz · 2021-12-22T14:32:22Z

Lines 35 to 41 in 41979ba

    
           if (this.commitViable()) { 
        
               this.commitMessageLength(); 
        
               this.bangPrefix(); 
        
               this.lowerCaseAfterPrefix(); 
        
               this.signedOffBy(); 
        
               this.moreThanAHyperlink(); 
        
           }

The requirements are similar to ours but there are differences, so it's moot to discuss individual ones. Just the above seems a bit odd:

at first, you might get a reply that says your message would be too short
- you might end up writing a longer message, but you still get the same reply
- because it is not checking for message length at all but for additional lines instead
The above condition can be satisfied, alone by adding signed-off
Only after having satisfied the initial condition (msg length), you get the others, e.g. : capitalization
- make you wonder: "hey, why didn't it say that in the first place?"

I just wonder why it's done that way..

webstech · 2021-12-28T07:15:51Z

The original intent was to check if a commit was viable based on minimal linting. If it passed that, all the other checks were run, so they would all report issues. The viable checks assumed if the minimal check was not met, more checking would probably fail.

Can this be adjusted for your needs (and linting requirements) or is a plug-in framework needed (that could help with generic project support as well).

Not sure if this answers your issue. If a PR is viable, all lints should run. If they don't, we messed up conversion from asynch to synch methods (my opinion).

softworkz · 2021-12-28T07:25:34Z

My point is that the logic is flawed:

You can't report that the message would be "too short" from a check that doesn't measure the message length but the line count.

The other point is:

The user should be told right away what he needs to change instead of getting a vague message first and only afterwards the actual required changes.
The effect is that a user needs to change and re-push each commit message twice, where the user will say:

"Hey, why didn't you tell me right away what you want?"

The solution is simple:
commitViable() should be removed and all would be good .

softworkz · 2021-12-28T07:38:46Z

Can this be adjusted for your needs (and linting requirements) or is a plug-in framework needed (that could help with generic project support as well).

I think a configuration file (see #829) would be sufficient where you can specify like:

check1: true,
check2: false,
check3: true,
check3_maxLen: 80,

softworkz · 2021-12-28T07:53:46Z

My point is that the logic is flawed:

I'm not nit-picking about hypothetical situations. I've gone through this myself when I submitted a (real) patchset as PR for testing:

I ended up duplicating the commit message texts to satisfy the conditions
I force-pushed
then I got new errors about missing signed-off text
I added the signed-off lines to all commit messages
force-pushed again

then I realized that I wouldn't have been required to increase the text lengths at all. the addition of a signed-off line alone is silencing the "message too short" check. And the annoyed user has been me in that case. ;-)

dscho · 2022-01-05T14:19:07Z

The user should be told right away what he needs to change

I agree with this except with one important aspect. It should not be he who should be told. You are single-handedly trying to brush away half of humanity, the legacy of the original coders, as well as (probably inadvertently) manifesting that awful gender bias that haunts the software industry. Please don't. Use "they", as Shakespeare would have. Don't pretend that every human being worth being a user of GitGitGadget is male.

softworkz · 2022-01-05T19:13:44Z

I considered this as an informal conversation among 3 developers, not an official text ;-)
Otherwise I'm more careful in wording.
Still not a friend of things like "Student*InnenFutter", though...

webstech · 2022-01-05T20:01:22Z

The user should be told right away what needs to change

I agree with this

I am reworking commit-lint.ts to do all the checks possible for a one line message.

Regarding the needs of another project, would it be better to allow a plugin to replace the linter? This could be loaded at run-time via configuration. Might be simpler than trying to configure which checks and check values are needed.

softworkz · 2022-01-05T20:06:59Z

We don't have much different requirements, so wouldn't need a plugin mechanism.

The only change I made is this (the last block) - and that's it..

    private commitMessageLength(): void {
        const maxColumns = 76;
        if (this.lines[0].length > maxColumns) {
            this.block(`First line of commit message is too long (> ${
                maxColumns} columns): ${this.lines[0]}`);
        }

        if (this.lines[1].length) {
            this.block("The first line must be separated from the rest by an "
                        + "empty line");
        }

        for (let i = 2; i < this.lines.length; i++) {
            if (this.lines[i].length > 72) {
                this.block("Please wrap lines in the body of the commit " +
                    "message between 60 and 72 characters.");
                break;
            }
        }
    }

softworkz · 2022-01-05T20:30:02Z

Some simple parametrization would do it IMO, like:

const COMMITLINT_MESSAGEWRAPCHECK_ENABLED = true;
const COMMITLINT_MESSAGEWRAPCHECK_MAX_COLS = 72;
const COMMITLINT_MESSAGEWRAPCHECK_MSG = "Please wrap " +
          "lines in the body of the commit message " +
          "between 60 and 72 characters.";

dscho · 2022-01-06T15:31:59Z

Still not a friend of things like "Student*InnenFutter", though...

And I am really appalled by the disrespect for non-male people. I have no patience for that. If you want to be respected, respect others first.

softworkz · 2022-01-06T23:03:26Z

You got me wrong. I highly respect all human beings equally and I'm one of the most tolerant people toward all kinds of concepts of life and existence. Also I admit that saying "he" in the context above is not ideal for a common audience and I usually avoid this. I just disagree with recent proposals to language changes (especially German). This has nothing to do with one's general attitude towards life, people and individual humans of whatever kind, though.
As we don't know each other in person, it's difficult to transport a full picture in two sentences, But I can assure you, that if we would, you would not have the slightest doubt, that I might disrespect any human being on this planet. Respecting other people is an attitude that needs to be learned, taught, spread and shared with other people in daily situations, whether its 1-to-1 or 1-to-many. Standing in for unknown persons even against your best friends, is one example that helps turning weights of viewpoints that others might have into a different balance - that's something that everybody can and should do inside one's own microcosm.
From my experience, this is something that does work and has impact.
An enforced (rather than naturally evolved) change to a language is not a suitable way and the discussion about it even has an adverse effect on society. That's my point of view in a nutshell 😄

dscho · 2022-01-09T12:54:44Z

It sounds a bit disingenuous if a representative of an over-represented gender tries to defend that they don't want to change their gendered language, even if they know that it is wrong. I am sure you can do better.

softworkz · 2022-01-09T13:55:33Z

What you consider better is what I consider worse - seems we both got strong positions regarding language evolvement..

dscho · 2022-01-10T12:44:38Z

The problem I see here is that you fight for your convenience. And I fight for other people to get the respect they deserve. Not quite on an equal footing, those positions, I'd say.

softworkz · 2022-01-10T13:32:47Z

I don't fight. And my reasons are multifold, convenience is none of them. I respect your opinion, I value your passion and I concur with the intention. The difference is that I disagree to the way. Hyper-imposed language changes are causing a split in society and have a strong adverse effect of rejection which people are eventually projecting on the whole subject.
Also, at a higher level, this is heading into a wrong direction from my point of view. What should stand at the end is unconditional equality of people, no matter where they are from, who they are or which gender one might have.
Starting to differentiate gender in language is a wrong and unsuitable measure for achieving this.

Another difference is that I'm not making despicable assumptions about your motivations in every single reply.

webstech mentioned this issue Jan 7, 2022

lint: run all possible checks on short commit msg #841

Merged

softworkz mentioned this issue Jan 7, 2022

Factoring Out all Project Specific Values #829

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commit Checks #818

Commit Checks #818

Commit Checks #818

Commit Checks #818

Comments