Adding Get Repository.Contents functionality

[Shazwazza]

I'm unsure of if you will accept PRs that only implement one part of the functionality. For example, for the Repository.Contents there is: Get, Create/Update, Delete, Get archive link

But I'm really only looking to implement the Get part. Is that ok for a phase 1 approach?

Please let me know what else needs to be considered, i haven't looked at the tests yet so I'm unsure if everything is tested or not.

This PR was replaced by #146

[msftclas]

All CLA requirements met.

[Shazwazza]

I've marked this 'ready' for review since I have this working locally with this code for retrieving file contents, you can pass in -Path to get a single item or leave it out to get the root.

[HowardWolosky]

Thanks for the submission, @Shazwazza! Welcome to the project.

There's certainly no requirement for a submission to include coverage for all API's within a relevant area. All I ask is that any provided function covers all potential functionality that the API would support for that function, which yours does.

I've called out a number of things that need to be fixed in order to be merged in, but they're mostly pretty minor and shouldn't take much time to address. I would also ask that you include a couple UT's along with your change (testing retrieving the contents of this repo's README.md (as a single file) and the whole contents (as a directory) should be sufficient.

In terms of actual usage though, I'm finding that your method is returning back unexpected results.

I tried this:

Get-GitHubContents -OwnerName Microsoft -RepositoryName PowerShellForGitHub -Path README.md -MediaType Raw

and it returned back to me an array of integers (likely representing each character). Whereas, if I call what should be the equivalent command:

Invoke-GHRestMethodMultipleResult -Description 'Getting Contents' -UriFragment /repos/Microsoft/PowerShellForGitHub/contents/README.md -AcceptHeader 'application/vnd.github.v3.Raw'

Which gives me back the full response. It's not clear to me what is different between those two commands, but I'll leave it to you to figure out and resolve that issue.

Finally, I suggest that include an additional switch that will auto-decode the encoded file contents for users, given how that is a fairly likely scenario.

Thanks again for the contribution! Looking forward to seeing what you do next!

[HowardWolosky]

There isn't anything that we really gain here from logging Path. There's no change in behavior that we'd make with that data, so given that, we wouldn't log it. What would be interesting would be to log something that differentiates usage for getting a single file vs a directory of files. But Path by itself doesn't need to be logged.

[Shazwazza]

For this logging it's not really possible to differentiate between whether it's a file or a directory since it's just a name and GitHub figures out if it's a file or directory so we'd only know that based on the results. What do you suggest in this case, should i just remove the Path parameter from being logged? Or do you have another idea?

[Shazwazza]

@HowardWolosky thanks for the notes! A bunch of this was copy/pasted (guilty!) so yup there's certainly some left over things lying around. I'll get back to this as soon as i can, cheers!

[Shazwazza]

Had pushed some changes and then started writing/running unit tests... unfortunately the GitHubRepositoryForks.tests.ps1 test will delete any fork of this repo that you currently have :P maybe not the most ideal thing when trying to contribute, haha.

I don't think there's another option here except for me to create a new fork and PR and close this one unless you know some other way?

[Shazwazza]

Have pushed a new PR here #146