Skip to content

Fix a debug mode bug#74

Merged
albertalef merged 7 commits into
albertalef:masterfrom
gerases:debug-mode-bug
Mar 10, 2026
Merged

Fix a debug mode bug#74
albertalef merged 7 commits into
albertalef:masterfrom
gerases:debug-mode-bug

Conversation

@gerases

@gerases gerases commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

When executing a command like so:

RubyShell.debug = true
sh do
  <some command>
end

The debug mode was ignored due to the logic of the debug method.

When executing a command like so:

```
RubyShell.debug = true
sh do
  <some command>
end
```

The `debug` mode was ignored due to the logic of the `debug` method.
@albertalef

Copy link
Copy Markdown
Owner

Hi @gerases! I will review this PR in some hours, i was in a work travel. Sorry for the delay!

@gerases

gerases commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

no worries

@albertalef

Copy link
Copy Markdown
Owner

Hi @gerases! Sorry again for the delay xD, now im back full steam ahead!

I wrote a Contributing Guideline.

Can you check your changes and update if something is needed?

@albertalef

albertalef commented Mar 10, 2026

Copy link
Copy Markdown
Owner

@gerases I made some changes here (because we added some new rules in rubocop.yml). Check if you agree and tell me, to we merge

@gerases

gerases commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Sure, but can you help me identify what specifically you would like me to focus on?

@albertalef

Copy link
Copy Markdown
Owner

The main change was make each "it" have only one expect. But i did already run the rubocop, and all is fine now. When you say ok, we can merge

@gerases

gerases commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, i'm definitely ok. Are you ok with my refactoring of the test in general? I decided to remove the duplication by using shared examples.

Not important, but curious, why are you preferring double quotes?

@albertalef

albertalef commented Mar 10, 2026

Copy link
Copy Markdown
Owner

About the shared_examples. No problem to me, i generally avoid shared examples because repeated expectations reveals a spoil, and only increase falsely the tests count, but in this situation the repeated expectations was necessary, because we want to check if the output of the debug is consistent. So, i dont think have a problem.

About the double quotes, we have a frozen_string_literal, so is not necessary use single quotes, and double quotes is only a convenience, because when is needed to add a string interpolation, is not needed to change the quotes for this xD, nothing beyond this, only for practicality.

@albertalef albertalef merged commit 03d702e into albertalef:master Mar 10, 2026
2 checks passed
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.

2 participants