-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
wcrtomb probe added, perl changes need to be done; need advice and README changes #52
Comments
For commits like this I do not really care. Sane and simple! BUT ...
|
On 01/12/2018 07:58 AM, H.Merijn Brand wrote:
For commits like this I do not really care. Sane and simple!
BUT ...
1. There is a format error in the ?S: section (spaces instead of tabs).
I'll commit the fix.
I ran the tools. Did they show an error for you?
2. Why did you choose the threads/ folder? Reading the probe does not
hint me to it being thread related
No one needs reentrancy unless it is a threaded-build.
3. Copyright 2017? Shouldn't that be 2018?
Indeed.
… —
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#52 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGhn9Ioc1p31b8VbgF1tHZm3DdgMsAqks5tJ3MmgaJpZM4Rb2pR>.
|
• I saw no errors, but I remember from earlier days they will appear. Not sure if it was in ?S: or another section. Anyway, there should be a test for that, if not for any error, then at least for consistency |
On 01/12/2018 07:58 AM, H.Merijn Brand wrote:
For commits like this I do not really care. Sane and simple!
BUT ...
1. There is a format error in the ?S: section (spaces instead of tabs).
I'll commit the fix.
I copied that .U, and just changed some words around. My guess is that
my editor expanded the tabs, so we do need a test to prevent this. I
know you're busy, but I don't know what it should do, should it be
combined with an existing test, etc. If you can lay out some quick
requirements, I could implement it.
And I need to know if Win32 has this function implemented:
configthreadlocale().
According to MS docs, it came along in VC8. According to our docs, we
support back to VC6. That means I have to #ifdef code. I don't know if
a Configure probe should be written, or if the code should somehow look
at the VC version. What do you suggest, and can you point me to a
template as to how to do it?
My probe for thread-safe nl_langinfo_l is defective because some
platforms require xlocale.h to be included, and some don't even have
that hdr. It's not clear to me how to write a probe that handles this.
Again, I bet there is a template probe that does something similar, and
that you can quickly point me to one.
|
I already have that check. I'm currently waiting for an answer from Rafael on what I propose |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Again, I've apparently pushed something directly without doing a PR. I followed the instructions. I think I need to create a branch and then push that. But I haven't changed my workspace since the last time when it did work.
Anyway, the perl changes are at
https://perl5.git.perl.org/perl.git/shortlog/refs/heads/wcrtomb
The text was updated successfully, but these errors were encountered: