pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/gitgitgadget/gitgitgadget/pull/2161

l-52276e82f63bb403.css" /> Update WELCOME.md with CC list instructions by gitster · Pull Request #2161 · gitgitgadget/gitgitgadget · GitHub
Skip to content

Update WELCOME.md with CC list instructions#2161

Open
gitster wants to merge 1 commit intogitgitgadget:mainfrom
gitster:patch-2
Open

Update WELCOME.md with CC list instructions#2161
gitster wants to merge 1 commit intogitgitgadget:mainfrom
gitster:patch-2

Conversation

@gitster
Copy link
Contributor

@gitster gitster commented Mar 4, 2026

(see commit log message)

Clarified instructions regarding CC list placement in PR descriptions. Too many people mistakenly put them in their commit message but GitGitGadget does not pay attention to them.
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! A contribution by the Git maintainer Himself!

While I understand, and agree with, the motivation, the implementation should probably involve a more precise logic. Here is my suggestion:

Comment on lines +18 to +19
NOTE: The CC list discussed here is to be added to the PR (Pull Request) description, *NOT* in your commit messages!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. As far as PR comments go, the welcome message is already quite long. So long, in fact, that its cognitive load virtually guarantees that any new contributor will miss some important bit of it.

Also, it strikes me as the wrong layer to just lump this warning into the welcome message. A welcome message should be, well, welcoming. That is not exactly accomplished by shouting at everyone indiscriminately that they should not include CC: lists in their commits when the vast majority of the audience already avoids that.

A more appropriate layer to do this is most likely phase 2 of the commit linter, which already warns about missing Signed-off-by trailers. It should be relatively simple to check for CC lists, too, something along these lines should do the trick:

    private containsCCLines = (): void => {
        const ccLines = this.lines.filter((line) => line.match(/^(\s*)Cc:\s+(\S*@\S*\.)/i));

        if (ccLines.length > 0) {
            this.block([
                "Contains Cc: lines (which need to be moved to the PR description):",
                ...ccLines
            ].join("\n"));
        }
    };

It would be quite easy to verify that it works by adding a unit test similar to the one testing a commit message that misses the Signed-off-by: trailer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy