Re: [NonGNU ELPA] New package: hyperdrive-org-transclusion

[Philip Kaludercic]

Joseph Turner <joseph@ushin.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Joseph Turner <joseph@ushin.org> writes:
>>
>>> Hello all,
>>
>> Hey, sorry for the delay,
>>
>>> <https://git.sr.ht/~ushin/hyperdrive-org-transclusion> extends
>>> org-transclusion by allowing content served over the hyper:// network to
>>> be transcluded using hyperdrive.el.  
>>
>> Code/content-wise, I cannot really comment on anything as it is based on
>> multiple layers of API that I am not familiar with.  That being said,
>> the (display-fill-column-indicator-mode) in the .dir-locals.el file is
>> very unusual and something I think you should avoid.  Also, an
>> .elpaignore file that would exclude some of the files that are only
>> interesting to contributors and maintainers would be nice.
>
> Thank you for the review!!
>
> The (display-fill-column-indicator-mode) addition was something that we
> settled on in the hyperdrive.el project last year as a way to remind
> contributors to fill code before committing.  I'm open to removing it.
> Would you elaborate on the reasons to avoid it?

The main reasons is that it involves an `eval' expression, which is
inherently controversial, especially when enabling a visual minor mode.

Have you considered using Git hooks to enforce the style guide?

>>> + (hyperdrive-org-transclusion :url 
>>> "https://git.sr.ht/~ushin/hyperdrive-org-transclusion";
>>> +  :readme "README.org"
>>
>> This file is automatically detected, we just choose to ignore README.md
>> files by default.
>
> Good to know.  I've removed that line in the attached patch.
>
>>> +  :news "CHANGELOG.org")
>>
>> Fun fact:  If you call the file NEWS.org, it will be automatically detected.
>
> Good to know.  I'd like to leave CHANGELOG.org as it is for now.

This is actually a fairly common file name, that it would be worth
checking for it as well.

> Thank you!!
>
> Joseph
>
>
> From 931e466053437a727fea76005b368f436cbf80e9 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph@breatheoutbreathe.in>
> Date: Sun, 22 Sep 2024 22:46:57 -0700
> Subject: [PATCH] * elpa-packages (hyperdrive-org-transclusion): New package
>
> ---
>  elpa-packages | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index 0ea8b159a6..3ddbfec540 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -402,6 +402,9 @@
>    :release-branch "stable"
>    :doc "doc/hyperdrive.org")
>  
> + (hyperdrive-org-transclusion :url 
> "https://git.sr.ht/~ushin/hyperdrive-org-transclusion";
> +  :news "CHANGELOG.org")
> +
>   (idle-highlight-mode        :url 
> "https://codeberg.org/ideasman42/emacs-idle-highlight-mode";
>    :ignored-files (".elisp-autofmt"))

I'll go ahead an apply this change, as the local variable issue isn't
something I'd consider to be "blocking".

-- 
        Philip Kaludercic on siskin