Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

automate cleaning up deprecated io/ioutil #377

Merged
merged 3 commits into from Aug 25, 2022
Merged

automate cleaning up deprecated io/ioutil #377

merged 3 commits into from Aug 25, 2022

Conversation

galargh
Copy link
Contributor

@galargh galargh commented Aug 25, 2022

Following up on #366 (comment)

I tested the newly added part locally on ipfs/go-ipfs-files (note, that particular repo will require manual intervention anyway because it uses ioutil.ReadDir too but I think most other repos don't).

Merging to master because it will only affect the repos with unmerged upgrades to Go 1.19.

@galargh galargh requested a review from a team as a code owner August 25, 2022 11:14

# As of Go 1.19 io/ioutil is deprecated
# We automate its upgrade here because it is quite a widely used package
if [[ $TARGET_VERSION == "1.18" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

This if is always true, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. I wanted to make it clear that we can remove it in time for the next upgrade but I guess a comment might be more fitting for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably shouldn't remove it. If any of the updates is not merged (and the repo stays on an old uCI version), it would be nice if this was included in the next update, just in case. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, we might also want to enrol a new repository that's not on latest Go yet and still uses ioutil - it'd come in handy then too.

Let me just remove the if-clause then.

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @galargh!

Does it make sense to try this out with one or two repos first?

@galargh
Copy link
Contributor Author

galargh commented Aug 25, 2022

Does it make sense to try this out with one or two repos first?

Definitely! I was going to use testing branch for that - will merge if everything looks good.

@galargh
Copy link
Contributor Author

galargh commented Aug 25, 2022

Tested on:

@galargh galargh merged commit 329d778 into master Aug 25, 2022
@galargh galargh deleted the ioutil branch August 25, 2022 12:10
masih pushed a commit that referenced this pull request Nov 8, 2022
* automate cleaning up deprecated io/ioutil

* remove target version guard and add goimports install

* install goimports by providing the tag name
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants