search

ckan / ckanext-spatial

Geospatial extension for CKAN
http://docs.ckan.org/projects/ckanext-spatial
126 stars 193 forks source link

#188 Add clean_tags option in harvester #189

Closed etj closed 6 years ago

etj commented 6 years ago

Fix #188 Fix #190 Fix #169

etj commented 6 years ago

(PR to be squashed, of course :) . I left all the commits in to preserve the original author).

etj commented 6 years ago

Still working on fixing tests...

etj commented 6 years ago

@amercader this PR is on the same line of ckan/ckanext-harvest#303 and ckan/ckanext-dcat#104 but for CSW harvesters, so I guess this PR can be useful as well. Any feedback? TIA

tdipisa commented 6 years ago

Hello @amercader, do you have some feedbacks about this PR ?

tdipisa commented 6 years ago

Dear @amercader, do you have some news about this PR? Are there some possibilities to merge it?

amercader commented 6 years ago

The changes look good, the test is a bit hard to follow, perhaps just adapt one of the existing ones to check the tags? Also I recommend that you look into the tests factories provided in CKAN core to make easier to create datasets, users, etc (ckan.tests.factories)

etj commented 6 years ago

Hi @amercader, the fixes made according to your review have been merged in this PR.

amercader commented 6 years ago

@etj @tdipisa can you merge master? The tests are fixed there so we should know reliably if this ones are passing. Thanks

tdipisa commented 6 years ago

Hello @amercader, done :)

amercader commented 6 years ago

Thanks @tdipisa and @etj!