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.

Sign up for GitHub

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

Jump to bottom

Replace deprecated ioutil usage with the equivalent replacements #198

Merged
merged 2 commits into from Apr 29, 2024
Merged

Replace deprecated ioutil usage with the equivalent replacements #198

merged 2 commits into from Apr 29, 2024

Conversation

james0209
Copy link
Contributor

@james0209 james0209 commented Apr 26, 2024 •
edited

What

Since Go 1.16 and Go 1.17, ioutil usage has been deprecated (most of the funcs call the os or io equivalents behind-the-scenes.

This PR just replaces the ioutil usage with the equivalent replacements.

ioutil.Discard -> io.Discard
ioutil.TempDir -> os.MkdirTemp
ioutil.ReadFile -> os.ReadFile
ioutil.TempFile -> os.CreateTemp
ioutil.ReadAll -> io.ReadAll

The PR also lints the Markdown files.

@hashicorp-cla-app HashiCorp CLA App
Copy link

hashicorp-cla-app bot commented Apr 26, 2024 •
edited

CLA assistant check
All committers have signed the CLA.

@hashicorp hashicorp deleted a comment from hashicorp-cla-app bot Apr 26, 2024
radeksimko
radeksimko reviewed Apr 29, 2024
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

I'm happy with the ioutil related changes but I'd appreciate if you could moved out the markdown changes, perhaps into a separate PR.

We can discuss any comments concerning markdown there.

README.md Outdated

## Example Usage

See examples at https://pkg.go.dev/github.com/hashicorp/hc-install#example-Installer.
See examples at <https://pkg.go.dev/github.com/hashicorp/hc-install#example-Installer>.
Copy link
Member

Choose a reason for hiding this comment

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

I struggle to find the meaning of this <URL> syntax - do you mind sharing link to any relevant docs to help me understand the reason for the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use markdownlint to format Markdown

docs/CONTRIBUTING.md Outdated
2. Wait for [`build` workflow](https://github.com/hashicorp/hc-install/actions/workflows/build.yml) to finish
3. Run the Release workflow with the appropriate version (matching the one in `version/VERSION`) & SHA (long one).
4. Wait for a message in the Slack channel saying that authorisation is needed to promote artifacts to production. Click on the link and approve.
5. Once notified that promotion is successful, go to <https://github.com/hashicorp/crt-workflows-common/actions/workflows/promote-production-packaging.yml>, locate the hc-install promote-production-packaging workflow, and approve.
Copy link
Member

Choose a reason for hiding this comment

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

I don't mind the whitespace removal but I would strongly prefer the indexes to stay as-is, i.e. 1. The reason is that if a human goes to add another item into the middle of the list, they have to re-index any following numbers.

Also similar question here about the <URL> syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, I understand that logic

@james0209
Copy link
Contributor Author

Hey @radeksimko, yeah I'll remove the Markdown changes from this PR and open them up in another 😄

radeksimko
radeksimko approved these changes Apr 29, 2024
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the move.

@radeksimko radeksimko merged commit 7566c14 into hashicorp:main Apr 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radeksimko radeksimko radeksimko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@james0209 @radeksimko