New/Set-GitHubRepository: Add Support for Merge Commit Title and Message Options

[X-Guardian]

Description

This PR adds merge commit message and title parameters to the New-GitHubRepository and Set-GitHubRepository functions:

Issues Fixed

• Fixes New/Set-GitHubRepository: Add Support for Merge Commit Title and Message Options #384

References

• https://docs.github.com/en/rest/repos/repos#create-an-organization-repository
• https://docs.github.com/en/rest/repos/repos#update-a-repository

Checklist

• You actually ran the code that you just wrote, especially if you did just "one last quick change".
• Comment-based help added/updated, including examples.
• Static analysis is reporting back clean.
• New/changed code adheres to our coding guidelines.
• Formatters were created for any new types being added.
• New/changed code continues to support the pipeline.
• Changes to the manifest file follow the manifest guidance.
• Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
• Relevant usage examples have been added/updated in USAGE.md.
• If desired, ensure your name is added to our Contributors list

[X-Guardian]

@HowardWolosky, this PR is ready for review.

[HowardWolosky]

I appreciate this change. Sending this one back to you for changes to the enum values though, as they aren't consistent with PowerShell naming conventions.

[HowardWolosky]

I prefer using PowerShell casing syntax for parameters, and then converting as necessary within the function for what the API requires. So, instead of PR_TITLE, it would be PRTitle and instead of MERGE_MESSAGE it would be MergeMessage. Similar for the other ones.

You can then use a hashtable to cleanly handle the conversions:

e.g.

$squashMergeCommitTitleConversion = @{
    'PRTitle' = 'PR_TITLE'
    'MergeMessage' = 'MERGE_MESSAGE'
}

# ....

$hashBody['squash_merge_commit_title'] = $squashMergeCommitTitleConversion.$SquashMergeCommitTitle

Similar feedback throughout.

[X-Guardian]

If we do this, then I think we really need to modify all GitHub.Repository objects returned by any function so that the Merge Commit property values match, otherwise this will be confusing to users of the module. If we change the current property values, merge_commit_title for example, then that is technically a breaking change. I could add additional properties, MergeCommitTitle for example with the Pascal case value, but then we are duplicating the properties.

[X-Guardian]

@HowardWolosky, any response to this?

[HowardWolosky]

Apologies for the delayed response here.

Great question -- I appreciate you bringing it up.

My general philosophy on this module is that I don't want to modify the raw return values that GitHub is providing, but that I will add properties that either add clarity/value, or assist in the continued functioning of the module.

Given that, I wouldn't want to modify the merge_commit_title / merge_commit_message values returned by GitHub. I'd be totally fine with adding a MergeCommitTitle/MergeCommitMessage/etc... property to the object when the equivalent GH property exists, using the naming/casing syntax that is part of the module, even if that makes the properties duplicative.

On this one, I don't feel super strong on needing those extra properties though. I don't think that the different casing/underscore usage on the GH object property will necessarily be confusing to users, but I can be easily swayed here. I'd happily approve a change that added the additional properties if the source ones existed in the GH object. I just want to keep the parameters consistent in casing convention. The additional properties on the object have been a bit more important to me when those could be used as parameters when piped into a different module function.

[X-Guardian]

I've update the parameter names as requested.

[HowardWolosky]

Thanks for this change, and apology for review delay. Two simple typos and then a fairly straightforward module coding style request. Otherwise, looks ready to go.

[HowardWolosky]

From a module style perspective, the preference is that all script-level variables are used the following way:

• References to them should explicitly include the $script: to make it clear where they come from
• Should be SentenceCased
• The should be read-only unless there's a good reason otherwise (e.g. being used to cache data)

We have the convenience creator at the top of the file to make those read-only script-level variables. So, you can do something like this in there for these:

{
    # ... cut for brevity
    GitHubRepositoryTeamPermissionTypeName = 'GitHub.RepositoryTeamPermission'
    SquashMergeCommitTitleConversion = @{
        'PRTitle' = 'PR_TITLE'
        'CommitOrPRTitle' = 'COMMIT_OR_PR_TITLE'
        'MergeMessage' = 'MERGE_MESSAGE'
    }
    # ... and the next one, etc...
 }.GetEnumerator() | ForEach-Object {
     Set-Variable -Scope Script -Option ReadOnly -Name $_.Key -Value $_.Value
 }

[X-Guardian]

Done.

[HowardWolosky]

/azp run PowerShellForGitHub-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[HowardWolosky]

Looks great. Thanks for applying those updates!
Kicking off CI now.

[HowardWolosky]

/azp run PowerShellForGitHub-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).