Repository Contents API #649
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -80,6 +79,7 @@ public interface IObservableRepositoriesClient | |||
| /// <param name="owner">The owner of the repository</param> | |||
| /// <param name="name">The name of the repository</param> | |||
| /// <returns></returns> | |||
| [Obsolete("This method has been obsoleted by Contents.GetReadme. Please use that instead.")] | |||
There was a problem hiding this comment.
Since the property below now is called Content instead of Contents, maybe these should be changed as well? 😄
|
Because we're pre 1.0, I'm inclined to just remove these methods rather than obsolete them. |
|
Could someone try this out? I think it's ready to merge. 🍰 |
|
I'll have a look at this when I'm back on the grid tomorrow ✌️ |
| /// </summary> | ||
| public string Content | ||
| { | ||
| get { return EncodedContent.FromBase64String(); } |
There was a problem hiding this comment.
Should this guard against null for EncodedContent?
|
Code and tests look pretty great. Just a few nagging questions 🤘 |
Some APIs return either an object or an array. This is hard for us static typed languages. So what we do is normalize things to an array when we expect one.
But only ones with ParameterAttribute applied.
haacked
force-pushed
the
haacked/repository-contents
branch
from
0b329f2 to
ca1ab1e
Compare
And fix whitespace
haacked
force-pushed
the
haacked/repository-contents
branch
from
ca1ab1e to
d769125
Compare
|
Something keeps fucking with the whitespace. I suspect it's the documentation rewriter. Anyways, this should be good I hope. I'm up too late again. 😄 so I'm going to sleep and hope the build is green. 📦 |
|
|
This was referenced Mar 10, 2015
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This incorporates and supersedes #434
I took a slightly different approach. In the interest of keeping close to the API, there's only one method called
GetContentsthat maps to the "Get contents" API. I was able to accomplish this by changing the serializer a bit. When it's trying to deserialize a JSON object into anIEnumerable<T>but the JSON text is clearly aTand not an array, I simply wrap the json text with square brackets turning it into an array of one item and then deserialize it.The other bit I had to do is when creating content, the text of the content needs to be base64 encoded. Also, when requesting content, the data is base64 encoded. So I changed the serializer to handle this transparently by simply applying the
SerializeAsBase64attribute. So the consumer of the client doesn't have to worry about this.