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

Extract blob upload resume into its own method #2930

Merged
merged 1 commit into from Jun 26, 2019
Merged

Extract blob upload resume into its own method #2930

merged 1 commit into from Jun 26, 2019

Conversation

dmathieu
Copy link
Contributor

I've found this logic being in a single method to be quite hard to get.
I believe extracting it makes it easier to read, as we can then more easily see what the main method does and possibly ignore the intricacies of ResumeBlobUpload.

@dmathieu
Copy link
Contributor Author

cc @caervs

Copy link
Contributor

@caervs caervs left a comment

Choose a reason for hiding this comment

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

Yeah that is clearer. LGTM.

@caervs caervs requested a review from manishtomar May 24, 2019 04:01
I've found this logic being in a single method to be quite hard to get.
I believe extracting it makes it easier to read, as we can then more
easily see what the main method does and possibly ignore the intricacies
of `ResumeBlobUpload`.

Signed-off-by: Damien Mathieu <dmathieu@salesforce.com>
@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #2930 into master will increase coverage by 0.01%.
The diff coverage is 37.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2930      +/-   ##
==========================================
+ Coverage   60.45%   60.47%   +0.01%     
==========================================
  Files         102      102              
  Lines        8002     8006       +4     
==========================================
+ Hits         4838     4842       +4     
  Misses       2515     2515              
  Partials      649      649
Flag Coverage Δ
#linux 60.47% <37.5%> (+0.01%) ⬆️
Impacted Files Coverage Δ
registry/handlers/blobupload.go 45.81% <37.5%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79f6bcb...9409751. Read the comment docs.

@caervs
Copy link
Contributor

caervs commented Jun 26, 2019

@dmathieu Thanks for resolving merge conflicts

@caervs caervs merged commit ec84b86 into distribution:master Jun 26, 2019
@dmathieu dmathieu deleted the extract-blob-resume branch June 26, 2019 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants