inspirehep / refextract

Extract bibliographic references from (High-Energy Physics) articles.
GNU General Public License v2.0
131 stars 31 forks source link

made CFG_PATH_GFILE and CFG_PATH_PDFTOTEXT configurable from the environment #29

Closed erickpeirson closed 7 years ago

erickpeirson commented 7 years ago

In order to containerize refextract for use in a web service, I needed to be able to explicitly specify the location of the pdftotext binary. So I altered refextract/references/config.py slightly so that the environment variables CFG_PATH_GFILE and CFG_PATH_PDFTOTEXT are preferred over shutil.which. This may be relevant for #9.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.009%) to 79.022% when pulling e3efc23c57db18c0efafef9317847d7fb7c5ea2f on erickpeirson:master into ec837cb4fc796dbe14fc6c9fc5d516026a706a3f on inspirehep:master.

jacquerie commented 7 years ago

Woah, someone else is actually using refextract ? Great!

Commit looks ok, except for the commit message. What about

config: make file and pdftotext paths configurable

Makes `CFG_PATH_GFILE` and `CFG_PATH_PDFTOTEXT` configurable
through shell variables, with fallback on the output of `which`,
in order to allow for easier containerization.

Signed-off-by: Erick Peirson <your.email@example.org>

?

erickpeirson commented 7 years ago

That seems like overkill. And it's already "signed off" as I'm the commiter. So, no thanks; but that sounds like a great line for the release notes! :-)

On Jun 20, 2017, at 4:29 AM, Jacopo Notarstefano notifications@github.com wrote:

Woah, someone else is actually using refextract ? Great!

Commit looks ok, except for the commit message. What about

config: make file and pdftotext paths configurable

Makes CFG_PATH_GFILE and CFG_PATH_PDFTOTEXT configurable through shell variables, with fallback on the output of which.

Signed-off-by: Erick Peirson your.email@example.org — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

jacquerie commented 7 years ago

That's the thing, we generate the release notes from the commit messages!

As for the Signed-off-by, eh, we had this debate internally, and we have people on your side and people that like the trailers for things like Co-authored-by to share credits. But ok, I'd rather have your commit than not. Can we compromise on

config: make file and pdftotext paths configurable

Makes `CFG_PATH_GFILE` and `CFG_PATH_PDFTOTEXT` configurable
through shell variables, with fallback on the output of `which`,
in order to allow for easier containerization.

or even

config: make file and pdftotext paths configurable

?

jacquerie commented 7 years ago

Ping @erickpeirson: if you answer the above I can include the commit in the next release...