Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply triggers to existing datastore resource #3565

Merged
merged 2 commits into from May 18, 2017

Conversation

fanjinfei
Copy link
Contributor

@fanjinfei fanjinfei commented May 15, 2017

Fixes #
A system administrator level API to actively apply triggers to existing datastore resource without end user's action.

Proposed fixes:

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@amercader amercader self-assigned this May 16, 2017
@amercader
Copy link
Member

Hi @fanjinfei many thanks for this. In general can you provide a short description of what the changes introduced do in the PR description and the commit messages? This is useful when other developers check the code in the future and want to know the context of them and what to expect.

I don't think that the function name describes well what it does. If I understood correctly, something like datastore_run_triggers would me more accurate right?

And on the docstring, perhaps replace:

The datastore_trigger_each_row API action allows you to apply triggers to an existing DataStore resource.

with

The datastore_run_triggers API action allows you to re-apply existing triggers to an existing DataStore resource.

WDYT?

@fanjinfei fanjinfei changed the title update datastore with triggers apply triggers to existing datastore resource May 16, 2017
@amercader
Copy link
Member

Thanks @fanjinfei

@amercader amercader merged commit 782c9a5 into ckan:master May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants