Commit messages should not be fully lowercased

[jonahsnider]

How are you running Renovate?

WhiteSource Renovate hosted app on github.com

Please select which platform you are using if self-hosting.

No response

If you're self-hosting Renovate, tell us what version of Renovate you run.

No response

Describe the bug

When Renovate opens a PR with semantic commits the entire commit message/PR title is lowercased. This is unnecessary, only the first letter needs to be lowercased (per Angular commit guidelines, I think most other semantic commit specs do the same).

Given the title:

fix(deps): Update My Org dependencies

Actual behavior:

fix(deps): update my org dependencies

Expected:

fix(deps): update My Org dependencies

Relevant code: https://github.com/renovatebot/renovate/blob/00e191795ce9a56752ece1891b41d0b6b92cba77/lib/workers/repository/updates/generate.ts#L184-L189

Relevant debug logs

No response

Have you created a minimal reproduction repository?

No reproduction repository

[rarkins]

What specific text in the linked angular document are you referring to?

[jonahsnider]

<type>(<scope>): <short summary>

[...]

Use the summary field to provide a succinct description of the change:

• [...]
• don't capitalize the first letter

From "Commit Message Format" > "Commit Message Header" > "Summary"

[rarkins]

We need to make sure our findPr function is case insensitive before we make this change.

[RahulGautamSingh]

I have already worked on the specified file and function while fixing #12025 , I can update this function and close this issue with #12025. I would like work on this while awaiting feedback on my other PRs.

[Xavientois]

@RahulGautamSingh I see that https://github.com/renovatebot/renovate/pull/12025 is now merged. Were you able to fix this as part of that pull request?

[RahulGautamSingh]

@Xavientois I am sorry for the delay. There is one folder lib/workers remaining to be merged, cause I haven't created a PR for it yet. I have completed the work but there has been a blackout in my city for last 3 days due to a rainstorm, so I haven't been able to use my laptop.

The work is completed just the commit & push is remaining.

Also it seems I linked the wrong PR, I meant that this issue will be closed with #11875 .

[RahulGautamSingh]

We need to make sure our findPr function is case insensitive before we make this change.

export async function findPr({
  branchName,
  prTitle,
  state = PrState.All,
}: FindPRConfig): Promise<Pr | null> {
  let prsFiltered: Pr[] = [];
  try {
    const prs = await getPrList();

    prsFiltered = prs.filter(
      (item) => item.sourceRefName === getNewBranchName(branchName)
    );

    if (prTitle) {
      prsFiltered = prsFiltered.filter((item) => item.title === prTitle);
    }

    switch (state) {
      case PrState.All:
        // no more filter needed, we can go further...
        break;
      case PrState.NotOpen:
        prsFiltered = prsFiltered.filter((item) => item.state !== PrState.Open);
        break;
      default:
        prsFiltered = prsFiltered.filter((item) => item.state === state);
        break;
    }
  } catch (err) {
    logger.error({ err }, 'findPr error');
  }
  if (prsFiltered.length === 0) {
    return null;
  }
  return prsFiltered[0];
}

findPr function seems to be case insensitive. Can I make changes to the code snippet highlighted by @jonahsnider?

[rarkins]

Isn't item.title === prTitle a case sensitive check?

[RahulGautamSingh]

Isn't item.title === prTitle a case sensitive check?

My bad. I guess we have to use localCompare() instead.

[rarkins]

Or a couple of couple of toLowerCase()

[RahulGautamSingh]

https://github.com/renovatebot/renovate/blob/a13da7ed74186aa69e2e206e41b0ceaea7a9e9dc/lib/workers/repository/updates/generate.ts#L197-L210

Do we need to change this too?

[rarkins]

Depends where/why the lower case field is set.

[RahulGautamSingh]

Depends where/why the lower case field is set.

I think we might have change the upgrade.prTitle = upgrade.prTitle.toLowerCase(); cause the toLowerCase field of upgrade will be true in case a commit follows all rules. https://github.com/renovatebot/renovate/blob/5a330336768e77906ec1e1422d96deb5bf0fa179/lib/workers/repository/updates/generate.ts#L163-L166

[rarkins]

So we need to get smarter than simply lower or not lower case

[RahulGautamSingh]

@rarkins

 if (upgrade.toLowerCase) {
        const titleMessage = upgrade.prTitle.split(' ');
        titleMessage[0] = titleMessage[0].toLowerCase();
        upgrade.prTitle = titleMessage.join(' ');
        upgrade.prTitle = upgrade.prTitle.toLowerCase();
      }
    } else {
      [upgrade.prTitle] = upgrade.commitMessage.split('\n');
    }

===

 if (upgrade.toLowerCase) {
      // We only need to lowercase the first line
      const splitMessage = upgrade.commitMessage.split('\n');
      let firstMessage: string[];
      if (splitMessage.includes(':')) {
        firstMessage = splitMessage[0].split(':')[1].split(' ');
      } else {
        firstMessage = splitMessage[0].split(' ');
      }
      firstMessage[0] = firstMessage[0].toLowerCase();
      splitMessage[0] = firstMessage.join(' ');
      splitMessage[0] = splitMessage[0].toLowerCase();
      upgrade.commitMessage = splitMessage.join('\n');
    }

These are the changes I came up with. I am trying on a regex solution but the regex becomes to complex and uses lookarounds in it, which RE2 doesn't support.

I ran into one problem while running build workflow on the updated files.

Jest: "global" coverage threshold for statements (100%) not met: 99.98%
Jest: "global" coverage threshold for lines (100%) not met: 99.98%

I tried looking into it last time but couldn't find a solution, could you please help me with solving this one?

• The errors occur when I use the new code, coverage is 100% incase of old code.

[rarkins]

Is the only reason we do lower casing for semantic commits?

[RahulGautamSingh]

Is the only reason we do lower casing for semantic commits?

It seems so. To get more details we will have test it by opening a PR and then the check details in CodeCov site. I am not familiar with coverage so I cannot be sure.

[rarkins]

It looks like upgrade.toLowerCase is only set due to semantic commits. So we should rename it to be e.g. upgrade.semanticCommitCasing and afterwards do a more intelligent lower-casing (of only the first letter, not all)

[RahulGautamSingh]

It looks like upgrade.toLowerCase is only set due to semantic commits. So we should rename it to be e.g. upgrade.semanticCommitCasing and afterwards do a more intelligent lower-casing (of only the first letter, not all)

New code:-

 if (upgrade.semanticCommitCasing) {
      // We only need to lowercase the first word
      const splitMessage = upgrade.commitMessage.split('\n');
      splitMessage[0] = splitMessage[0].replace(
        splitMessage[0].includes(':') ? regEx(/: \w+/) : regEx(/^\w+/),
        (match) => match.toLowerCase()
      );
      upgrade.commitMessage = splitMessage.join('\n');
    }

 if (upgrade.semanticCommitCasing) {
        upgrade.prTitle = upgrade.prTitle.replace(regEx(/^\w+/), (match) =>
          match.toLowerCase()
        );
      }

[RahulGautamSingh]

Hey @rarkins , do you approve of this suggestion by @pret-a-porter ?

[viceice]

In general yes, but I hope we don't need to pass the CommitMessage instance around.

I hope it can be used in a single function where the message is currently generated. And then the ready message is used as now.

[RahulGautamSingh]

@viceice I have made the certain changes to the commit-message model and added test cases for the same. I was wondering if I should check for empty commit messages or not, if yes what should I return in such case.

public static formatCasing(commitMessage: string): string {
    return commitMessage.replace(
      regEx(/^([\w]+:|[\w]+\(\w+\):)/).test(commitMessage)
        ? regEx(/: \w+/)
        : regEx(/^\w+/),
      (match) => match.toLowerCase()
    );
  }

Changes made to this file

Added these test cases

  it('should format commit message casing', () => {
      expect(CommitMessage.formatCasing('fix: Upgrade something')).toBe(
        'fix: upgrade something'
      );
      expect(CommitMessage.formatCasing('fix(docs): Something else')).toBe(
        'fix(docs): something else'
      );
      expect(CommitMessage.formatCasing('fix: Upgrade Something')).toBe(
        'fix: upgrade Something'
      );
      expect(CommitMessage.formatCasing('Really Great feature: Foo')).toBe(
        'really Great feature: Foo'
      );
    });

I am unsure of the last test, incase it's wrong please reply the expected value and I will modify the code according to it.

[pret-a-porter]

@rarkins Please, assign this one to me

[matty-rose]

hi is there any update on the status of this issue?

[renovate-release]

:tada: This issue has been resolved in version 32.84.1 :tada:

The release is available on:

• GitHub release
• 32.84.1

Your semantic-release bot :package::rocket:

[viceice]

• #16054
• #16057

[viceice]

@pret-a-porter Needs revert do to bugs. Please check / fix the issue and open a new PR.

[renovate-release]

:tada: This issue has been resolved in version 32.99.3 :tada:

The release is available on:

• GitHub release
• 32.99.3

Your semantic-release bot :package::rocket:

[kingcrunch]

Hi,

We really need this feature. So if somebody can have a look at his, we would really appreciate it. I for myself will try, but don't expect too much, because I am more a Go/Rust guy ^^

[viceice]

we should review @pret-a-porter PR again, which then needs a lot real tests to not break things again.