jekyll-remote-theme icon indicating copy to clipboard operation
jekyll-remote-theme copied to clipboard

Propagate _data folder from remote theme

Open mgerzabek opened this issue 3 years ago • 24 comments

All files from _data folder within remote theme are accessible in consumer projects. The behaviour is exactly the same as for overrides of _includes and _layouts. Data files with the same name in consumer project override data files within remote theme. As supposed by @hblankenship and @desima in #68


View rendered README.md

mgerzabek avatar Mar 08 '21 08:03 mgerzabek

Welcome! Congrats on your first pull request to Jekyll Remote Theme. If you haven't already, please be sure to check out the contributing guidelines.

welcome[bot] avatar Mar 08 '21 08:03 welcome[bot]

Thanks for this @mgerzabek. Can you provide a bit more detail as to the use case where a theme should be injecting data into the consumer site?

benbalter avatar Mar 08 '21 16:03 benbalter

Well, I have a catalogue of text modules for a layout file concerned with GDPR. And I want them to be in the remote theme to have changes to this catalogue be available for all projects that use my theme.

Further so, I see that in putting a catalogue in a (remote) theme becoming a curated collection of possibly anything would give a better user experience/consumer experience of themes.

mgerzabek avatar Mar 08 '21 18:03 mgerzabek

My current remote theme has icons stored in _data that have to be manually copied to work.

datapolitical avatar Mar 12 '21 11:03 datapolitical

@duboisp we could finally use the _data folder to store theme settings and variables

delisma avatar Mar 12 '21 18:03 delisma

Since this is my first pull request and I couldn’t find a reviewer, may I ask @benbalter how the stage of this PR could be pushed forward.

As I understand from the timeline I need a review from someone with write access to this repo. Unfortunately the Reviewers section is empty, even the owner of the repo is not listed, so I cannot ask for a review. Is this some kind of a chicken-egg-related problem?

Any help/clearing words appreciated.

mgerzabek avatar Apr 07 '21 08:04 mgerzabek

Another use case would be i18n files within the theme that can be accessed by the site using the theme. Just one of many, many cases. For myself, we use a remote theme for our foundation website that would store things like events which the foundation website and multiple sub-websites would then display. Our use-case is somewhat unusual: theme - main website - sub-site (separate repository but 'same' website - sub-site (as above) - sub-site (as above) - etc.

example case: https://owasp.org Each project and chapter, for instance, is its own 'site'(repository) within our website but they are all linked together. If I want information to show up on each sub-site, it should reside in the theme. Please merge this.

OWASP Foundation, the Open Source Foundation for Application Security on the main website for The OWASP Foundation. OWASP is a nonprofit foundation that works to improve the security of software.

hblankenship avatar May 21 '21 19:05 hblankenship

I think this could be a really useful feature for all Jekyll themes. Would you consider submitting the change to the Jekyll project upstream?

parkr avatar Jul 23 '21 17:07 parkr

Would you consider submitting the change to the Jekyll project upstream?

Mhm… okay, I‘m just a Jekyll end user and not a ruby pro. Beside that, of course, I’d do it, but I‘m not sure where to start. Any hints where and how to go about it?

mgerzabek avatar Aug 27 '21 04:08 mgerzabek

@benbalter What do we need to do to get this PR merged? A review?

hblankenship avatar Aug 27 '21 14:08 hblankenship

From what I understand, yes. A review from someone who can do this. Until yesterday the checks where green, then I klicked the merge button to synchronize the PR with the latest sources and now even the checks don‘t run with the message “Merging can be performed automatically with 1 approving review.“ But this is my first PR so don’t take my word for it…

mgerzabek avatar Aug 28 '21 07:08 mgerzabek

For those interested in this feature and deploying their site using GitHub Actions, there's a plugin that adds this feature to Jekyll: jekyll-data

ashmaroli avatar Aug 31 '21 09:08 ashmaroli

With the success of PR 8815 for Jekyll this PR becomes obsolete. When v4.3 is released the desired behaviour should be within Jekyll standard distro. Closing this.

mgerzabek avatar Nov 22 '21 14:11 mgerzabek

@mgerzabek It would be good to validate that. It's my understanding that at least the Theme class in this repo may need updating to support the data loading from the theme.

parkr avatar Nov 22 '21 18:11 parkr

@parkr you’re right. Thanks for the hint! I’ll check this as soon as a Jekyll release with the merged changes from the referenced PR are live. For now I reopen this PR.

mgerzabek avatar Nov 25 '21 09:11 mgerzabek

I’ll check this as soon as a Jekyll release with the merged changes from the referenced PR are live.

@mgerzabek You need not wait for an official release. You may point your test-site's Gemfile to the Jekyll repository and work on an implementation when possible:

# Gemfile

gem "jekyll", github: "jekyll/jekyll"
...

ashmaroli avatar Nov 25 '21 09:11 ashmaroli

@parkr + @ashmaroli rolled back to the minimal required changes to this PR to work with the new additions for Jekyll.

Also sorry for the noise… I enhanced my local copy of this PR to circumvene the downloading of remote theme when it’s present at my local machine and didn’t realize this morning that I have to redo these changes. Everything should be fine now.

mgerzabek avatar Nov 25 '21 16:11 mgerzabek

Mhm… okay, not sure if I‘m able to deliver this… I created a new PR for the Primer theme. It adds a data file greetings.yml with one single variable.

As soon as this PR get’s merged into the theme I could then continue to see how a test would be implented within this repo.

mgerzabek avatar Dec 09 '21 11:12 mgerzabek

As soon as this PR get’s merged into the theme

@mgerzabek I don't see why you have to wait for a downstream merge.. Your own fork should suffice for low-key testing..:

# _config.yml
remote_theme: mgerzabek/primer@add_data_folder_for_remote_theme

ashmaroli avatar Dec 09 '21 11:12 ashmaroli

I this actually still needed with Jekyll 4.3.2? With me it already works since the remote theme download and extracts the whole theme repo in temp, and the theme source is set to Theme source: /tmp/jekyll-remote-theme-20230831-18672-p0x48d, it already is reading out the _data files, since it is a 4.3 feature.

bedroesb avatar Aug 31 '23 09:08 bedroesb