Re: [RFC] Module code and POD for CPAN

Hello Bod,

is long to read it carefully, so just some notes.

$attrs{'currency'} //= 'GBP'; defined or was intruduced in 5.10.0 so you need use 5.010; in your code and in your Makefile.PL or what you use instead of it. If you want to support older perl versions you need an alternative syntax like: $attrs{'currency'} = $attrs{'currency'} ? $attrs{'currency'} : 'GBP' ;

my %attrs = @_; you put some type checking some line below. I like it splitted into another, dedicated sub: my %attrs = _check_args(@_); here you can excercise your fantasy at will. Wrong args can be common for a public used module.

Carp is another things I like a lot. Generally a misuse by a module consumer should point to code invoking and provoking the error, not to your module code at line ... Internal errors of your module should be highlighted more carefully, possibly pointing both to your and their code.

Searching for attrs in your code I noticed something strange: your methods get_intent get_intent_id get_ids all starts with my ($self, %attrs) = @_; I do not understand: only get_ids is called with arguments. Then (but I do not looked to it closer) why these attrs are not incapsulated inside the main object?

Internal usage of HTTP_HOST and the alike should be documented. I dont recall the whole thing, but document it. They are always used by every server? Only by some? (I'm not a web expert as you can see ;)

Finally I like your pod because it explains nicely what the matter is: this is a rare quality!

It appears you are using CGI as all your example use it: in 2021 one might expect some example at least in Dancer2

Then, but is my lack of understanding, I dont get why your module ends returning some html. I think you try to explain it in the pod but I dont get it :)

Do you plan to test your module against a live sandbox api? In author tests or during make test aka everytime someone install your module? This can be fun :)

There are no rules, there are no thumbs..
Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.

Comment on Re: [RFC] Module code and POD for CPAN Select or Download Code

Replies are listed 'Best First'.
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 11:11 UTC
Thank you so much for your helpful feedback A few question about them if you don't mind... If you want to support older perl versions... This is something I haven't looked at yet. I have looked at what version of the dependencies are required which are v0.014 of HTTP::Tiny and v2.00 of JSON::PP. Do you think I should support Perl versions older than v5.10 given that Perl v5.12's 11th birthday was 2 days ago!? If I decide not to support versions prior to v5.10, does the declaration need to go in the module or just in Makefile.PL? Searching for attrs in your code I noticed something strange: your methods get_intent get_intent_id get_ids all starts with my ($self, %attrs) = @_; I do not understand: only get_ids is called with arguments. Then (but I do not looked to it closer) why these attrs are not incapsulated inside the main object? You have highlighted a typo and a shortcoming in the clarity of the documentation! `get_intent`, `get_intent_id` and `get_ids` (not `get_id` as the documentation says!) all take the optional parameters `reference` and `email` (users may or may not want to use these parameters depending on their use case). Only `get_ids` optionally takes two other parameters: `public-api` and `format</i>. Controlling the output format between JSON and text is only relevant with this method. The <code>public-api` is needed by this method but not by others and needs to be passed in either here or at instantiation. I imagine users will pass both keys to the `new` method but having the option gives users the choice. Internal usage of HTTP_HOST and the alike should be documented. I dont recall the whole thing, but document it. They are always used by every server? Only by some? I'm really glad you picked this up! I had to do quite a bit of research to find what to do here and I still didn't come up with a definitive answer...this is why there is a TODO at the top of the code to make this more robust and why it checks for callback URLs in the constructor. Within my own environment I have historically used `SCRIPT_URI` but every now and then it disappears for reasons I don't understand - CGI environment variables seem to be bit of a dark art so I have deliberately used ones that seem to be standard. I don't have an instance of IIS to test it on. Having said that, I cannot imagine a situation outside of initial testing, where the end user will not want to send the customer to a different page depending on whether they paid successfully or not. So `cancel-url` and `success-url` would be explicitly set. It appears you are using CGI as all your example use it: in 2021 one might expect some example at least in Dancer2 Internally we use a bespoke decoder of query strings and form data so I had to pick something generally available. I picked CGI because I thought it was core (it isn't any more), I know the (basic) syntax and it is simple to understand. Perhaps Dancer2 would be better in the examples but I only put the examples in to show how easy this module is to use and to give people a headstart. I dont get why your module ends returning some html Thanks - I shall improve the documentation to explain this better	[reply] [d/l] [select]
Re^3: [RFC] Module code and POD for CPAN by Discipulus (Canon) on Apr 14, 2021 at 15:31 UTC
Hello Bod > Do you think I should support Perl versions older than v5.10 given that Perl v5.12's 11th birthday was 2 days ago!? If I decide not to support versions prior to v5.10, does the declaration need to go in the module or just in Makefile.PL? I dont think :) This is up to you. Some ancient systems still run perl versions with mustache and beards. Not being my opinion so much relevant, I assume 5.10 or 5.14 as my own personal backward compatibility limit. That said I suppose you must put it in both place, but probably `Makefile.PL` is enough being the first thing run during installation. My last module has it stated twice: `use 5.010; use strict; use warnings; use ExtUtils::MakeMaker; WriteMakefile( ... MIN_PERL_VERSION => '5.010', ... },` [download] I suspect the first one directly aborts the installation (as the file does not compile) and the second can/should generate a report. The redundance does not hurts me. > take the optional parameters reference and email .. Here also I'd put some check from a banal existence of a `@` to the usage of Regexp::Common::Email::Address > I don't have an instance of IIS to test it on The code is meant to run on IIS? I have made something with it and is a pain. Every other solution is better also handling HTTP requests by hand with pen and paper :) Eventually Apache can run on windows, some `plack` servers too. I'm a windows sysadmin: my best wishes. > CGI.. Well.. sincerely seing CGI example can make people fly in horror. Anyway I think no new projects use CGI nowadays. There are valid alternatives and mainly Mojolicious and Dancer2. Being in your cloths I'd propose a CGI example as the last one or ones. Queryin parameters is easy and readable with Dancer2 `get '/hello/:name?' => sub { my $name = route_parameters->get('name') // 'Whoever you are'; return "Hello there, $name"; };` [download] > Having said that, I cannot imagine a situation outside of initial testing, where the end user will not want to send the customer to a different page depending on whether they paid successfully or not. So cancel-url and success-url would be explicitly set. Reasoning a bit I dont like the solution. Your module should only do the operation needed to interact with this Stripe. I'm not a serious web dev but I think you should avoid it. In Dancer2 or Mojolicious you have the default route `/` and is the same of your `SCRIPT_NAME` +/- I suspect will be a pain refactor this part after the module is in production: rethink it if needed. > $attrs{'error'} = ... In the constructor your code returns only the last one error set. Then I prefere `undef $attrs{error}` over setting it to an empty string in the rest of the module. L* There are no rules, there are no thumbs.. Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.	[reply] [d/l] [select]
Re^4: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 22:26 UTC
The code is meant to run on IIS? I have made something with it and is a pain Who am I to predict where it will get run other than on a webserver of some kind or another. More and more low-cost web hosts seem to be offering a Windows environment that it's only a matter of time... I was looking up IIS to try and fathom what CGI environment variables I would be safe to use.	[reply]