`Sys-Hostname`: Few minor improvements
- Correct indirect syntax in test
- Correct indentation on
BEGIN - Enrich documentation
- Spelling and grammar
- Capitalize comment sentences
Bump version
While there are some things I am okay with in this pull request, it is trying to do too many things at once -- most of which don't need to be done at all.
There is no real benefit to changing the casing within comments, e.g.,
# method 1 - we already know it
to
# Method 1 - We already know it```
There are approximately 15 such changes in this patch. Not only are these changes useless, they also distract one's attention from the one truly substantive change in the patch, namely, the grammatical correction in the error message (from does not accepts to does not accept). I am in favor of correcting mistakes in our error messages, even at the cost of incurring some downstream breakage.
As @Grinnz has noted, we will also face some downstream breakage in changing Cannot get host name of local machine to Cannot get the hostname of the local machine. The only reason I'm not completely opposed to this change is that hostname is well known as a standard Unix command.
Until this p.r. is trimmed down to what we can agree is substantive, it should not be accepted.
While there are some things I am okay with in this pull request, it is trying to do too many things at once -- most of which don't need to be done at all.
There is no real benefit to changing the casing within comments, e.g.,
# method 1 - we already know itto
# Method 1 - We already know it```
The intended benefit is that when one reads the source code, they can sectionalize the "methods" easier.
While there are some things I am okay with in this pull request, it is trying to do too many things at once -- most of which don't need to be done at all.
There is no real benefit to changing the casing within comments, e.g.,
# method 1 - we already know itto
# Method 1 - We already know it```There are approximately 15 such changes in this patch. Not only are these changes useless, they also distract one's attention from the one truly substantive change in the patch, namely, the grammatical correction in the error message (from
does not acceptstodoes not accept). I am in favor of correcting mistakes in our error messages, even at the cost of incurring some downstream breakage.As @Grinnz has noted, we will also face some downstream breakage in changing
Cannot get host name of local machinetoCannot get the hostname of the local machine. The only reason I'm not completely opposed to this change is thathostnameis well known as a standard Unix command.Until this p.r. is trimmed down to what we can agree is substantive, it should not be accepted.
So basically you're saying that the PR can be accepted if I revert the comment case changes?
I wonder why PR's should include only substantive changes, and why are unsubstantive changes distracting reviewers (might just ignore harmless "trivial cosmetics")?
Super minimal changes in individual PR's will result in multiple version bumps for the distribution, and probably multiple CPAN syncing.
If it's neutral to you, neither good nor bad with no benefit or harm, let it be, no?
And I already mentioned above the reasoning behind those unsubstantive comment changes.
I also think this PR should have a Sys-Hostname GitHub label on it since it deals with one Perl distribution only 🏷️. I searched and it doesn't seem to exist, not sure why.
No opinion on the substantiveness of the changes, but regardless multiple version bumping is not necessary, since version bumps are only required when there has been a change from an existing release (in this case, since it is not dual life, a release of Perl).