Support in Set-GitHubContent to upload binary files

[StanleyGoldman]

Description

As mentioned in #335, this pull request adds functionality to upload binary content with Set-GitHubContent

Issues Fixed

References

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

[HowardWolosky]

Welcome to the project @StanleyGoldman, and thanks for the idea and for the submission. You're off to a great start. I've provided some feedback for you to take a look at when you get a chance.

Thanks again.

[HowardWolosky]

This needs to remain mandatory in its own ParameterSet.

[StanleyGoldman]

Fixed

[HowardWolosky]

I think there was a misunderstanding with how I phrased this. It does need to be in its own parameter set, but not at the exclusion of the other required information. For instance, in order to what repo we're changing the content of, we still need to have the information either in the Elements parameter set or the Uri parameter set.

Previously, Content was being used with either of those parameter sets. Now you are introducing ContentPath as an alternative to Content. That means we're now exploded from 2 possible parameter sets to 4:

• Elements with Content (maybe named ElementsContent)
• Elements with ContentPath(maybe namedElementsContentPath`)
• Uri with Content (maybe named UriContent)
• Uri with ContentPath (maybe named UriContentPath)

You should try running Get-Help -Name Set-GitHubContent and look at the Syntax section. That will show you the various parameter sets that PowerShell has understood from the authored code. If you look at that with what you've currently authored, you should see more clearly how your new Content parameter set excludes the key information that we need, as well as prevents Content and ContentPath from being used when the repo information is provided.

[HowardWolosky]

Take a look at Get-GitHubEvent. That's an example where we had to explode the number of ParameterSets in order to support both the Elements and Uri repo scenarios while still accurately separating out the different possible inputs for the method itself.

[HowardWolosky]

We may also want to allow this to be ValueFromPipeline to enable the content to be piped directly into this method. That's certainly an optional change for this PR though.

[StanleyGoldman]

Do you want to file an issue for this, or should I?

[HowardWolosky]

If you're not addressing this in this PR, then go ahead and open it up as a suggestion once this PR lands.

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 14 days of this comment.

[HowardWolosky]

Hey @StanleyGoldman,

Thanks so much for these updates, and my sincerest apologies for such a long time to get back to you with a follow-up review. I completely missed when you pushed up fixes earlier.

The main feedback here is still around the ParameterSets. Take a look at my more detailed comment in the reactivated comment thread to see if that gives you the additional information that you need to get it fixed.

[HowardWolosky]

If you're not addressing this in this PR, then go ahead and open it up as a suggestion once this PR lands.

[HowardWolosky]

I think there was a misunderstanding with how I phrased this. It does need to be in its own parameter set, but not at the exclusion of the other required information. For instance, in order to what repo we're changing the content of, we still need to have the information either in the Elements parameter set or the Uri parameter set.

Previously, Content was being used with either of those parameter sets. Now you are introducing ContentPath as an alternative to Content. That means we're now exploded from 2 possible parameter sets to 4:

• Elements with Content (maybe named ElementsContent)
• Elements with ContentPath(maybe namedElementsContentPath`)
• Uri with Content (maybe named UriContent)
• Uri with ContentPath (maybe named UriContentPath)

You should try running Get-Help -Name Set-GitHubContent and look at the Syntax section. That will show you the various parameter sets that PowerShell has understood from the authored code. If you look at that with what you've currently authored, you should see more clearly how your new Content parameter set excludes the key information that we need, as well as prevents Content and ContentPath from being used when the repo information is provided.

[HowardWolosky]

Suggested change

	The local path to file content.
	The local path to a file whose content should replace what is currently stored remotely in the repo at the Path location.

[HowardWolosky]

Take a look at Get-GitHubEvent. That's an example where we had to explode the number of ParameterSets in order to support both the Elements and Uri repo scenarios while still accurately separating out the different possible inputs for the method itself.

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 14 days of this comment.

[ghost]

This pull request has been automatically closed due to a lack of activity from the author. We understand. Life happens and other things likely came up. We would still love to see your contribution get merged in. Now that it has been closed, a different community member may wish to pick up where you left off. If so, they should speak up by commenting below. If you're still interested in completing this yourself, just respond back and let us know.