search

ckan / ckanext-pdfview

PDF viewer for CKAN
GNU Affero General Public License v3.0
26 stars 47 forks source link

[#23] Render resource view descriptions as Markdown. #24

Closed torfsen closed 7 years ago

torfsen commented 8 years ago

Fixes #23. This is actually the same fix as ckan/ckan#3129 (which fixes ckan/ckan#3128, the source of #23).

torfsen commented 8 years ago

The test failure doesn't seem to be related to my changes:

Installing the packages that CKAN requires...

sudo: must be setuid root

The command "bash bin/travis-build.bash" failed and exited with 1 during .

Your build has been stopped.

(also note that ckan/ckan#3129 passes the tests)

torfsen commented 8 years ago

Is there anything I can do to help bring this forward?

k-nut commented 8 years ago

Apparently the build is failing because Travis is trying to run this with Docker where sudo is not supported (but we need that for installing dependencies). According to this answer on stack overflow it should be possible to force the old, non-Docker based build system by specifing sudo: true in the .travis.yml. Maybe you want to do that and see if the build passes then?

torfsen commented 8 years ago

@k-nut Thanks for looking into this! I wasn't sure whether just fixing the Travis build was the right way to go, for example because core CKAN has abolished Travis in favor of CircleCI.

But since the fix seems to be so simple I just went ahead and made it. It's not related to the changes in this PR, however, so it has a separate one: #25.

k-nut commented 8 years ago

@torfsen It looks like almost all extensions still use travis so I guess that it makes sense to stick with that for now.

👍 for making the required changes on a separate branch. Lets hope that that fixes the build and we can merge this PR with confidence :)

amercader commented 7 years ago

@torfsen can you merge master so tests turn green?

torfsen commented 7 years ago

@amercader I've rebased onto master and added a test case while I was at it. Looks good now.