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

Fix TravisCI for https://github.com/Islandora-CLAW/CLAW/issues/643 re… #652

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

ruebot
Copy link
Member

@ruebot ruebot commented Jun 1, 2017

GitHub Issue: #63 (comment)

What does this Pull Request do?

Enables the rest Drupal module.

What's new?

Above.

How should this be tested?

Merge it and I'll restart TravisCI on the Drupal modules.

Interested parties

@dannylamb

@dannylamb dannylamb merged commit 31d465f into Islandora:master Jun 1, 2017
@ruebot ruebot deleted the fix-travis branch June 1, 2017 13:46
@jonathangreen
Copy link
Contributor

I wonder if we should be working out whatever dependancy issues we are having instead of forcing us to enable rest before we enable the Islandora module?

When I download a module I kind of expect to be able to drush en it without having to enable other modules first. Won't that be kind of confusing for folks installing islandora in the future?

Maybe I'm misunderstanding the issue. I'm always alarmed though, when we just force enable a module.

@dannylamb
Copy link
Contributor

@jonathangreen Ah just seeing this comment now. Yeah, this is meant to be a short-term fix. Let's keep #643 open until we get it sorted out properly.

For context, we need to modify the rest resource config for nodes that the rest module provides. We can't provide our own because the two will conflict. So I'm editing it in hook_install right now, which probably isn't the best place. I'm loathe to introduce a hook_update_N to do something not related to an update at all. Do you know of a better place to do it?

@whikloj
Copy link
Member

whikloj commented Jun 2, 2017

@dannylamb so is rest not enabled by our drupal-project fork? I'm not understanding what module is not enabled when we try this?

@dannylamb
Copy link
Contributor

@whikloj rest is installed when installing islandora since it's declared as a dependency. It can be pre-installed before installing islandora, so we needed to check that case as well.

But it's all moot at this point because I moved to using another hook. Instead of hook_install I'm using hook_modules_installed, which executes every time any module is installed, which covers both cases.

dannylamb pushed a commit to dannylamb/CLAW that referenced this pull request Feb 8, 2018
update API to include DS modified params
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.

4 participants