lib/Rex/Config.pm: slurp file content using IO::File. #1511
Conversation
monsieurp
force-pushed
the
1466_local-argv
branch
2 times, most recently
from
73badfc
to
a93c45b
Compare
|
Thanks @monsieurp for your contribution! Tis PR is still marked as draft only, but reading through the linked issue and the proposed changes, I can provide some early feedback below and/or as code comments.
Hmm, yeah, I agree it's probably fine to skip changelog entries for changes such as automated code formatting, refactoring, or CI workflow changes.
As a minimum quality requirement the current test suite should still pass. Ideally, if a changed code path is not covered yet, tests could be added first (to demonstrate a code is solid either by fixing a failing test to proving that it still passes the tests). Currently There are currently 35 instances of I've fixed some CI-related issues recently, so you might need to rebase this PR on top if current default branch, and (force) push your branch again. |
| my @lines = do { | ||
| use IO::File; | ||
| my $fh = IO::File->new( $config_file, 'r' ); | ||
| join '', $fh->getlines(); |
There was a problem hiding this comment.
Can it be that t/config-ssh.t fails because now all lines of the file is concatenated with join while it was treated as an array of lines before?
|
ps.: a git commit is explicit about which files are being changed, so there's no need to duplicate that info in the commit message. and to stop it with the period. Also do not end the subject line with a period. E.g. instead of:
it's fine to simply write:
It might also work better to explain what and why vs. how. E.g.
|
monsieurp
force-pushed
the
1466_local-argv
branch
from
a93c45b
to
eb6a7bb
Compare
monsieurp
force-pushed
the
1466_local-argv
branch
from
eb6a7bb
to
fc59cb9
Compare
monsieurp
force-pushed
the
1466_local-argv
branch
from
fc59cb9
to
037b3bb
Compare
Fixes #1466.
This PR is an attempt to fix #1466. The ticket is self-explanatory as well as those changes.
Checklist