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

[Philip Kaludercic <philipk@posteo.net>]

Joseph Turner <joseph@ushin.org> writes:

> Thank you for the review!
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>> There is really not much I can comment on, since it is mainly API I am
>> not familiar with.  One note is that the package description doesn't
>> make much sense to me, since I don't know what "transcluding" means.
>> Also, not everyone knows that "plz" is a HTTP library (the name remains
>> elusive), so a brief comment reminding the reader of that would be
>> appropriate.
>
> I've updated the package description.

This is the commit you are referring to?

diff --git a/org-transclusion-http.el b/org-transclusion-http.el
index 5f240ad31c..293e705615 100644
--- a/org-transclusion-http.el
+++ b/org-transclusion-http.el
@@ -25,8 +25,8 @@
 
 ;;; Commentary:
 
-;; This file contains functionality related to transcluding content over HTTP
-;; using plz.el.  Features include:
+;; This file extends the `org-transclusion' package to allow for transcluding
+;; content over HTTP.  Features include:
 ;;
 ;; - Transclude plain text
 ;;   + Transclude only Org headings matching search options

I don't know if this is just me, but I am not familiar with what
"transcluding" is supposed to mean.  I get that it has something to do
with networking, and if I had to guess you are fetching remote files and
including them while exporting, but a package description shouldn't
leave the reader with the impression that they have to guess what is
going on.

>> Another note is that if you already have a .dir-locals.el file, you
>> could add a few more variables that might vary from system to system.
>> In Compat, the .dir-locals.el lists:
>>
>> --8<---------------cut here---------------start------------->8---
>> ((nil
>>   (fill-column . 72)
>>   (sentence-end-double-space . t)
>>   (show-trailing-whitespace . t)
>>   (indent-tabs-mode . nil)))
>> --8<---------------cut here---------------end--------------->8---
>
> I've added all these variables except show-trailing-whitespace, as I
> don't personally use it.  ws-butler takes care of whitespace without
> adding visuals.

No problem, I like `show-trailing-whitespace' because I know that all
contributors will see that there is a whitespacing issue, but I admit
that my recommendations might be opinionated.

>> Also, I think an .elpaigore file would be appropriate to have in the
>> repo, at least to avoid bundling the build-time dependencies.
>
> I've added .elpaignore and pushed a new release v0.3.
>
> Thank you!!!
>
> Joseph
>
>

-- 
	Philip Kaludercic on peregrine