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

chore: replace usages of ioutil #1792

Merged
merged 1 commit into from
Oct 14, 2023
Merged

chore: replace usages of ioutil #1792

merged 1 commit into from
Oct 14, 2023

Conversation

donuts-are-good
Copy link
Contributor

ioutil is not used anymore, in favor of io or os. in this case, io is a drop in replacement for ioutil.

ioutil is not used anymore, in favor of io or os. in this case, io is a drop in replacement for ioutil.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congratulations on opening your first pull request! We'll get back to you as soon as possible. In the meantime, please make sure you've updated the documentation to reflect your changes and have added test automation as needed. Thanks! 🙏🏼

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (284066a) 69.23% compared to head (0a57f46) 69.23%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1792   +/-   ##
=======================================
  Coverage   69.23%   69.23%           
=======================================
  Files          26       26           
  Lines        2467     2467           
=======================================
  Hits         1708     1708           
  Misses        661      661           
  Partials       98       98           
Files Coverage Δ
pkg/container/client.go 50.12% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@donuts-are-good
Copy link
Contributor Author

I'm happy to help get this merged. Please advise what is needed.

@piksel
Copy link
Member

piksel commented Oct 12, 2023

Hello! Sorry, I have been a bit busy. The complaint is about how one of the changed lines has no test coverage, but this just because it had no coverage before. The PR is about replacing a call to a deprecated function with the non-deprecated version, so adding tests for the functionality might be a bit out of scope. Ideally you could add a test that "proves" that the new function does the same as before, but since it's just a rename/package move, that shouldn't be necessary.

@donuts-are-good
Copy link
Contributor Author

Hello! Sorry, I have been a bit busy. The complaint is about how one of the changed lines has no test coverage, but this just because it had no coverage before. The PR is about replacing a call to a deprecated function with the non-deprecated version, so adding tests for the functionality might be a bit out of scope. Ideally you could add a test that "proves" that the new function does the same as before, but since it's just a rename/package move, that shouldn't be necessary.

Awesome thanks! that's reassuring :) Is there anything else that's needed before this is merged or should I sit tight and wait?

@piksel piksel changed the title removes ioutil chore: replace usages of ioutil Oct 14, 2023
@piksel piksel merged commit 72e437f into containrrr:main Oct 14, 2023
11 of 12 checks passed
@donuts-are-good
Copy link
Contributor Author

Thanks :)

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