Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to get raw file content as byte[] #2151

Merged
merged 4 commits into from
Apr 12, 2020

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Mar 12, 2020

This allows to spare a little over 100ms in my tests on small files vs using the GetAllContentsByRef API.

Fixes #1651

@0xced 0xced force-pushed the raw-content branch 2 times, most recently from 42305ec to a1d690b Compare March 13, 2020 00:29
@codecov
Copy link

codecov bot commented Mar 13, 2020

Codecov Report

Merging #2151 into master will decrease coverage by 0.02%.
The diff coverage is 65.21%.

@@            Coverage Diff             @@
##           master    #2151      +/-   ##
==========================================
- Coverage   65.89%   65.86%   -0.03%     
==========================================
  Files         546      546              
  Lines       14251    14293      +42     
  Branches      836      838       +2     
==========================================
+ Hits         9390     9414      +24     
- Misses       4703     4720      +17     
- Partials      158      159       +1     
Impacted Files Coverage Δ
Octokit/Helpers/AcceptHeaders.cs 100.00% <ø> (ø)
Octokit/Helpers/ApiUrls.cs 97.51% <0.00%> (-0.23%) ⬇️
Octokit/Http/ApiConnection.cs 60.25% <0.00%> (-1.59%) ⬇️
Octokit/Http/Connection.cs 62.71% <0.00%> (-2.43%) ⬇️
...tive/Clients/ObservableRepositoryContentsClient.cs 94.39% <100.00%> (+1.05%) ⬆️
Octokit/Clients/RepositoryContentsClient.cs 100.00% <100.00%> (+0.92%) ⬆️
Octokit/Http/HttpClientAdapter.cs 78.26% <100.00%> (+0.19%) ⬆️
Octokit.Reactive/Helpers/ObservableExtensions.cs 90.29% <0.00%> (-2.92%) ⬇️

This allows to spare a little over 100ms in my tests on small files vs using the `GetAllContentsByRef` API.

Fixes octokit#1651
@0xced
Copy link
Contributor Author

0xced commented Apr 2, 2020

@shiftkey I just rebased onto master. Do you think you'll get some time to review this pull request sooner or later?

Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @0xced - I merged in the latest changes to ensure this is all working as expected, and tidied up some unnecessary changes rather than hold this up.

@@ -2363,7 +2363,7 @@ public static Uri RepositoryArchiveLink(string owner, string name, ArchiveFormat
/// <returns>The <see cref="Uri"/> for getting the contents of the specified repository and path</returns>
public static Uri RepositoryContent(string owner, string name, string path, string reference)
{
return "repos/{0}/{1}/contents/{2}?ref={3}".FormatUri(owner, name, path, reference);
return "repos/{0}/{1}/contents/{2}?ref={3}".FormatUri(owner, name, path == "/" ? "" : path, reference);
Copy link
Member

Choose a reason for hiding this comment

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

This is a nice bit of cleanup for my more explicit example above - thanks!

@shiftkey shiftkey merged commit 69d1182 into octokit:master Apr 12, 2020
@shiftkey
Copy link
Member

release_notes: added GetRawContent and GetRawContentByRef APIs to Repository Contents client for cases where performance is desirable

@0xced 0xced deleted the raw-content branch April 12, 2020 19:19
@0xced
Copy link
Contributor Author

0xced commented Apr 12, 2020

Thanks for merging and releasing version 0.47.0! 🎉

@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The client doesn't support for files raw content
3 participants