Remove specific \r\n code in TAP for Windows
Hi all,
$subject has been mentioned by Jacob (in CC.) on Discord, and it does
not seem like somebody took the time to propose a patch.
We use in some places of the TAP tests the following pattern:
my $newline = $windows_os ? "\r\n" : "\n";
Two files have this idea:
- 003_ldap_connection_param_lookup.pl
- 006_service.pl
Note: I have committed one of these.
However, in light of 1c6d4629394d, we should not require that, and the
CI looks happy with the attached.
That would be up to the buildfarm to act as final judge, but any
objections in attempting to get rid of these like in the attached?
I would try first HEAD to be sure, then follow with an optional
backpatch.
Thoughts?
--
Michael
Attachments:
0001-Remove-specific-CRLF-code-in-TAP-tests.patchtext/x-diff; charset=us-asciiDownload+13-32
On 24 Oct 2025, at 08:51, Michael Paquier <michael@paquier.xyz> wrote:
in light of 1c6d4629394d, we should not require that, and the
CI looks happy with the attached.
+1, a nice win in terms of readability.
--
Daniel Gustafsson
On Thu, Oct 23, 2025 at 11:51 PM Michael Paquier <michael@paquier.xyz> wrote:
$subject has been mentioned by Jacob (in CC.) on Discord, and it does
not seem like somebody took the time to propose a patch.
Ah, sorry, I put part of my first attempt at it over in [1]/messages/by-id/CAOYmi+kYKPXCfiAF3xAu+cHYSLYEc4OC9Wsh2nebwWboNbkpeQ@mail.gmail.com. But I
realize that's not very visible, nor does it address
003_ldap_connection_param_lookup.
I'm carrying a local diff that switches 003_ldap_ over to qq{} style.
Attached in case you like it, but your patch LGTM too.
That would be up to the buildfarm to act as final judge, but any
objections in attempting to get rid of these like in the attached?
I would try first HEAD to be sure, then follow with an optional
backpatch.
Sounds like a plan.
Thanks!
--Jacob
[1]: /messages/by-id/CAOYmi+kYKPXCfiAF3xAu+cHYSLYEc4OC9Wsh2nebwWboNbkpeQ@mail.gmail.com
Attachments:
003_ldap_newlines.diff.txttext/plain; charset=US-ASCII; name=003_ldap_newlines.diff.txtDownload+16-27
On Mon, Oct 27, 2025 at 08:33:57AM -0700, Jacob Champion wrote:
Ah, sorry, I put part of my first attempt at it over in [1]. But I
realize that's not very visible, nor does it address
003_ldap_connection_param_lookup.
Ahh, so you did post something. I have missed that
I'm carrying a local diff that switches 003_ldap_ over to qq{} style.
Attached in case you like it, but your patch LGTM too.
Your suggestions for the LDAP test are cleaner than what I came up
with, so reused these.
That would be up to the buildfarm to act as final judge, but any
objections in attempting to get rid of these like in the attached?
I would try first HEAD to be sure, then follow with an optional
backpatch.Sounds like a plan.
Time for a verdict, so applied on HEAD now.
--
Michael
On Tue, Oct 28, 2025 at 08:27:35AM +0900, Michael Paquier wrote:
Time for a verdict, so applied on HEAD now.
Three buildfarm members have reported after 8767b449a3a1: fairywren,
drongo and hamerkop. All of them have reported green, so applied the
equivalent cleanup on REL_18_STABLE with a92bbffbc3a7 for the service
test.
--
Michael
On Tue, Oct 28, 2025 at 6:08 PM Michael Paquier <michael@paquier.xyz> wrote:
Three buildfarm members have reported after 8767b449a3a1: fairywren,
drongo and hamerkop. All of them have reported green, so applied the
equivalent cleanup on REL_18_STABLE with a92bbffbc3a7 for the service
test.
Thank you!
--Jacob