perl5
perl5 copied to clipboard
Document various newSUB functions
On 5/10/22 07:25, iabyn wrote:
@.**** commented on this pull request.
Some general comments:
By upgrading a bunch of functions from 'Ap' to 'Apd', we are effectively making fully public, functions that used to be 'subject to change'. Are we sure we want to do this?
I'd actually prefer to leave them undocumented and mark them as internal. If that's ok with you, I'll do that instead.
I'd actually prefer to leave them undocumented and mark them as internal.
I'm not really sure. I've always been uncomfortable with the status of functions marked as API but with no documentation. I think it would be best to leave as-is for 5.36 then reconsider. I guess have a look at current usage (if any) on CPAN to see if people are using them to construct optrees etc, then decide whether to make them API or not.
@iabyn, here's what I think would be best for now. Leave the few that have cpan uses as-is, and mark the other ones as experimental. That makes them even more use-at-your-own-risk.
On 5/11/22 01:29, iabyn wrote:
I'd actually prefer to leave them undocumented and mark them as internal.I'm not really sure. I've always been uncomfortable with the status of functions marked as API but with no documentation. I think it would be best to leave as-is for 5.36 then reconsider. I guess have a look at current usage (if any) on CPAN to see if people are using them to construct optrees etc, then decide whether to make them API or not.
I'm uncomfortable with the status quo as well, so am trying to cut the number of undocumenteds down. These stood out as fairly easy pickings.
There is a bit of usage on CPAN, CV and SV only. 10 distros, but only 8 results showed; I don't know if that is a bug with metacpan grep or what.
https://grep.metacpan.org/search?q=new%5BACGHS%5DVREF%5Cs*%5C%28&qd=&qft=*.xs
We could also mark them as public but experimental, subject to change. But it seems to me that the ship has sailed, and at least those two need to be marked public, and I would lean towards the others too.
@iabyn do you still have concerns about this PR? @khwilliamson anything further needed here?
Resolved conflicts in embed.fnc
@iabyn do you still have concerns about this PR? @khwilliamson anything further needed here?
@iabyn, @khwilliamson, can you respond to @demerphq's questions ^^? Can this be merged?
@iabyn do you still have concerns about this PR? @khwilliamson anything further needed here?
@iabyn, @khwilliamson, can you respond to @demerphq's questions ^^? Can this be merged?
Again: @iabyn, @khwilliamson, can you respond to @demerphq's questions ^^? Can this be merged?
I'm not wild about it, but I guess its ok to merge.