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

Change IOGate back to GeneralIO #497

Merged
merged 2 commits into from
Jan 11, 2023
Merged

Change IOGate back to GeneralIO #497

merged 2 commits into from
Jan 11, 2023

Conversation

sshock
Copy link
Contributor

@sshock sshock commented Jan 10, 2023

To avoid side effects to other tests in ruby/ruby.

@sshock
Change IOGate back to GeneralIO
ba36067 
Staying with ANSI can cause side effects with other tests.
@sshock sshock mentioned this pull request Jan 10, 2023
Merged
st0012
st0012 reviewed Jan 10, 2023
View reviewed changes
Copy link
Member

@st0012 st0012 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't we reset the IOGate in Reline.test_reset? Now it's like

  1. Run test_mode
  2. Run the test
  3. Run test_mode again to unset just one of the setup it did
  4. Run test_reset

IMO this has some problems:

  1. Most of test_mode doesn't need to be run again but now it is.
  2. It confuses users about the intention of test_mode.
  3. Now we need 2 helpers to reset the test environment instead of just 1.

@sshock
Copy link
Contributor Author

sshock commented Jan 10, 2023

Shouldn't we reset the IOGate in Reline.test_reset?

Agreed and done!

@ima1zumi ima1zumi mentioned this pull request Jan 11, 2023
Closed
ima1zumi
ima1zumi approved these changes Jan 11, 2023
View reviewed changes
Copy link
Member

@ima1zumi ima1zumi left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

@ima1zumi ima1zumi merged commit 7479d83 into ruby:master Jan 11, 2023
@ima1zumi ima1zumi added bug Something isn't working and removed bug Something isn't working labels Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@st0012 st0012 st0012 left review comments

@ima1zumi ima1zumi ima1zumi approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sshock @st0012 @ima1zumi