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:

renovate/lib/workers/repository/updates/generate.ts

Lines 184 to 189 in 00e1917

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

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 #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]

renovate/lib/workers/repository/updates/generate.ts

Lines 197 to 210 in a13da7e

	if (upgrade.prTitle) {
	upgrade.prTitle = template.compile(upgrade.prTitle, upgrade);
	upgrade.prTitle = template.compile(upgrade.prTitle, upgrade);
	upgrade.prTitle = template
	.compile(upgrade.prTitle, upgrade)
	.trim()
	.replace(/\s+/g, ' ');
	// istanbul ignore if
	if (upgrade.prTitle !== sanitize(upgrade.prTitle)) {
	throw new Error(CONFIG_SECRETS_EXPOSED);
	}
	if (upgrade.toLowerCase) {
	upgrade.prTitle = upgrade.prTitle.toLowerCase();
	}

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.

renovate/lib/workers/repository/updates/generate.ts

Lines 163 to 166 in 5a33033

	upgrade.toLowerCase =
	regEx(/[A-Z]/).exec(upgrade.semanticCommitType) === null && // TODO #12071
	!upgrade.semanticCommitType.startsWith(':');
	}

[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 ?

[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.

[github-actions]

Hi there,

Get your issue fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible.

Before we start working on your issue we need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.

To get started, please read our guide on creating a minimal reproduction.

We may close the issue if you, or someone else, haven't created a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.

Good luck,

The Renovate team

[Xavientois]

@rarkins If this PR to fix the issue is soon to be merged, do we still need to provide a minimal reproduction?

[github-actions]

When a bug has been marked as needing a reproduction, it means nobody can work on it until one is provided. In cases where no reproduction is possible, or the issue creator does not have the time to reproduce, we unfortunately need to close such issues as they are non-actionable and serve no benefit by remaining open. This issue will be closed after 7 days of inactivity.

[github-actions]

This bug report has been closed as we need a reproduction to work on this. If the original poster or anybody else with the same problem discovers that they can reproduce it, please create a new issue, and reference this issue.

[HonkingGoose]

@rarkins If this PR to fix the issue is soon to be merged, do we still need to provide a minimal reproduction?

That PR is now merged:

• feat: commitMessageLowerCase #20930

I don't know if we still need a reproduction for the original problem though? I'll let @rarkins decide. Maybe we can close this as "done" instead of "not planned". 😄