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

Can't override core templates #3831

Closed
NateWr opened this issue Jun 26, 2018 · 6 comments
Closed

Can't override core templates #3831

NateWr opened this issue Jun 26, 2018 · 6 comments
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. Try Me This issue might be good for a new contributor. Can you help us?

Comments

@NateWr
Copy link
Member

NateWr commented Jun 26, 2018

In several places, templates call other templates with the core:<path> prefix. This prevents plugins from overriding these templates.

A quick scan suggests that we use it in several places where it is not necessary. We should remove the core: prefix from any template calls unless we're sure we never want this template overridden.

@NateWr NateWr added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Jun 26, 2018
@NateWr NateWr added this to To do in Nate's wee project via automation Jun 26, 2018
@NateWr NateWr added the Try Me This issue might be good for a new contributor. Can you help us? label Jun 26, 2018
@asmecher
Copy link
Member

@NateWr, the core: prefix is used to distinguish between application and pkp-lib repositories in cases where the same template might appear in both. Often an app-specific template will augment a built-in one using that approach.

NateWr added a commit to pkp/bootstrap3 that referenced this issue Aug 3, 2018
jamshidhashimi pushed a commit to jamshidhashimi/pkp-lib that referenced this issue Feb 4, 2019
jamshidhashimi pushed a commit to jamshidhashimi/pkp-lib that referenced this issue Feb 4, 2019
@jamshidhashimi
Copy link
Contributor

jamshidhashimi commented Feb 4, 2019

So wherever I saw core: is used, I checked if any such template exists in /templates/<path>. If it did, then I kept the core: prefix. Otherwise I removed the core: prefix.

PR:
#4416

Please review @asmecher @NateWr.

@asmecher
Copy link
Member

asmecher commented Feb 4, 2019

@NateWr, mind if I leave this one with you? Thanks!

@NateWr
Copy link
Member Author

NateWr commented Feb 5, 2019

Looks good, @jamshidhashimi! Can I get you to run the tests for OJS to ensure there's no regression here. In order to run the tests, you need to prepare a PR for OJS which adds the submodule and a special submodule commit. This will kick off the tests.

Here's a step-by-step:

  1. Get the latest master version of OJS.
cd <ojs-root-dir>
git checkout master
git pull upstream master
  1. Check out a new branch:
git checkout -b i3831_templates upstream/master
  1. Make sure the lib/pkp submodule has your branch checked out.
cd lib/pkp
git checkout i3831-fix
  1. Add the lib/pkp submodule and apply a special commit message that points to the remote/branch to test.
cd <ojs-root-dir>
git add lib/pkp
git commit -m "Submodule update ##jamshidhashimi/i3831-fix##"
git push -u origin i3831-fix

Then open a Pull Request on PKP's ojs repo with that branch, and add the link here. That will kick off the tests.

Once they pass, I can merge this. The last test in the set will always fail right now due to a bug (my fault). But once the tests finish just add a message here and I'll take a look.

If you have any difficulties with that process give me a shout.

@jamshidhashimi
Copy link
Contributor

Hi @NateWr. Thank you. Following your instructions, created the submodule update to trigger tests.

PR:
pkp/ojs#2266

@NateWr
Copy link
Member Author

NateWr commented Feb 6, 2019

Thanks @jamshidhashimi! Tests passed and I've merged it in. 👍

@NateWr NateWr closed this as completed Feb 6, 2019
Nate's wee project automation moved this from To do to Done Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. Try Me This issue might be good for a new contributor. Can you help us?
Projects
No open projects
Development

No branches or pull requests

3 participants