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

Fix: remove ioutil package #634

Closed
wants to merge 1 commit into from
Closed

Fix: remove ioutil package #634

wants to merge 1 commit into from

Conversation

fooofei
Copy link
Contributor

@fooofei fooofei commented Jul 19, 2023

Background

ioutil package is not recommand to use, instead use io package and os package

Checklist

  • Git commit messages conform to community standards.
  • Each Git commit represents meaningful milestones or atomic units of work.
  • Changed or added code is covered by appropriate tests.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@fooofei fooofei requested a review from tsenart as a code owner July 19, 2023 07:06
Copy link
Owner

@tsenart tsenart 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. Apart from the signal thing, I'll sign off this commit locally and push to main branch.

@@ -201,7 +201,7 @@ func attack(opts *attackOpts) (err error) {
res := atk.Attack(tr, opts.rate, opts.duration, opts.name)
enc := vegeta.NewEncoder(out)
sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt)
signal.Notify(sig, os.Interrupt, syscall.SIGTERM)
Copy link
Owner

Choose a reason for hiding this comment

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

This change seems unrelated. Accidental?

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 think SIGTERM is also commonly used, so I add it. Can we add it in this pr? If not, I will remove it.

Copy link
Owner

Choose a reason for hiding this comment

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

It's fine.

@tsenart tsenart closed this in 51bb5ee Jul 19, 2023
AlexanderYastrebov added a commit to AlexanderYastrebov/vegeta that referenced this pull request Aug 17, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
ioutil is deprecated, see https://go.dev/doc/go1.16#ioutil

Follow up on tsenart#634

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit to AlexanderYastrebov/vegeta that referenced this pull request Aug 17, 2023

Verified

This commit was signed with the committer’s verified signature. The key has expired.
doubledup David Dunn
ioutil is deprecated, see https://go.dev/doc/go1.16#ioutil

Follow up on tsenart#634

Signed-off-by: Alexander Yastrebov <[email protected]>
tsenart pushed a commit that referenced this pull request Aug 18, 2023
ioutil is deprecated, see https://go.dev/doc/go1.16#ioutil

Follow up on #634

Signed-off-by: Alexander Yastrebov <[email protected]>
Signed-off-by: Tomás Senart <[email protected]>
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