Re: Request to distribute Casual packages on NonGNU ELPA

[Philip Kaludercic]

Charles Choi <kickingvegas@gmail.com> writes:

> Philip -
>
> Appreciate the input. My responses to your feedback below:
>
>> My first impression that the project seems a tad over-engineered for
>
> This was a deliberate decision on my part in undertaking the
> development of Casual. As I am new to developing Elisp, I wanted to
> understand better what it would be like to build a library of Elisp
> packages using contemporary software engineering practices.

OK, it is just culturally uncommon for Elisp code (of the size as your
packages are) to be developed in such a way.

>> but I don't know where you got that information from?  (referencing ADM-3A)
>> https://www.emacswiki.org/emacs/EightyColumnRule e.g. says this goes
>> back to punch cards (which is the story I had in my head).
>
> My reference to the ADM-3A was written "off the cuff", as that was one
> of the first terminals I had worked with. I will amend to reference
> the 80 column rule.

1+

>> Regarding casual-lib.el:  Do you actually need Emacs 29?  Package lint
>> seems to be fine with lowering the version to Emacs 25.
>
> In specifying requirements, I've taken a more conservative tack of
> listing a configuration that I am able to test, in this case, Emacs 29
> running on macOS and Linux. I do not have the time nor resources to
> fully test older versions of Emacs and associated packages, much less
> test on different platforms.
>
> As I am new to Elisp publishing, I was and still am reluctant to trust
> lint tools to verify behavior on older versions of Emacs and
> associated packages as it would commit me to supporting them. Is lint
> sufficient enough for verification of correct behavior on lower
> versions of Emacs? What happens when it isn't?

Usually yes, but if not someone will tell you and you can adjust it.  As
insinuated above, Elisp development is more /casual/ and direct.  If
someone finds a problem despite what `package-lint' says, they are more
likely to report it to you in a constructive way.

>> + :group 'casual)                    ;please add a `defgroup' before 
>> referring to it!  You don't need to specify the :group afterwards.
>
> Will amend.
>
>> +(defun casual-lib-display-line-numbers-mode-p () ;why do you have this as a 
>> predicate?
>
> For reasons I do not understand or have clear enough knowledge about,
> I could not write an expression to pass to a Transient macro, but
> instead has to pass a function symbol to get working code. Hence
> making a predicate here.
>
>> +      (not transient--stack)))              ;btw. are you allowed to use 
>> this internal variable?
>
> This was guidance provided by Jonas Bernoulli, maintainer of Transient. 
> https://github.com/magit/transient/discussions/290

Then it's fine ^^  Though it would be nice if there were some explicit
API for that...

>> +  :key "C-q"                                ;IIUC this is the binding that 
>> closes your transient buffer?  Could this be rebound to the more 
>> conventional "q"?
>
> Initially I did. Guidance from Jonas Bernoulli argued that Transient
> convention is to use C-q as detailed in
> https://magit.vc/manual/transient/FAQ.html#Why-does-q-not-quit-popups-anymore_003f-1

Oh, I did not know about this.  Forget about that then.

>> +  (casual-lib-customize-casual-lib-hide-navigation)) ;why not just inline 
>> the above definition?
>
> Likely over-modularization on my part. Will audit all Casual packages and if 
> this usage is singular, will inline.

Great, I find that would help with readability.

>> +awk '/Version: / {print $3}' $1
>
> Nice in that I don't have to make a call to grep. Will verify and amend.
>
> Thanks for taking the time to review.
>
> Regards -
>
> Charles
>
>
> —
> Charles Y. Choi, Ph.D.
> kickingvegas@gmail.com
>
>

-- 
        Philip Kaludercic on siskin