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

Add instructions to add your own themes. #52

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ content/data/*.db
content/logs/
config.production.json
content/data/ghost-local.db

content/themes/*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content/themes/ need to be excluded because they're meant to be copied over from node_modules, NOT hardcoded in the repo.

Revert this change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaneJeon I don't think this should be a problem. See my points on why pushing your themes to this repository might be a better idea. Also, since the included themes are copied over to content/themes after the install step (which happens only on heroku), we should not have to worry about getting them there in content/themes in the source code

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course all the themes belong in content/themes, as specified by Ghost CMS.

The question is whether you want to check the raw theme code into your codebase, and that's really just a hacky AF solution. Come up with a better way (that doesn't involve checking in individual themes) or remove this change entirely.

!content/themes/.gitkeep
25 changes: 20 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,35 @@ This repository is a [Node.js](https://nodejs.org) web application that specifie
- Versions are locked and managed using [npm](https://www.npmjs.com/)
- Scales across processor cores in larger dynos via [Node cluster API](https://nodejs.org/dist/latest-v10.x/docs/api/cluster.html)


## Using a new theme

Due to the Ephimeral file system of Heroku, currently the admin panel doesn't support uploading the themes directly. If uploaded, the theme would go missing after some time. The only way to use custom theme currently is to commit your themes in the code files and then re publish the application.

**To use a new theme follow these steps**

1. Fork and clone this repository
2. Extract and copy your theme folder in content/themes. If your theme is `Editorial` it should look like `content/themes/Editorial/...`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, no. The correct way to do it is to host your own theme on github/gitlab/npm and then specify the versions in package.json. This entire section needs to be rewritten.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaneJeon I can see your point there but I still think this is a better way to host themes. Here are my reasons:

  1. The method of hosting my theme on GitHub and specifying versions in package.json has way too much complexity. The things you have to do to get it working are:
    • Create a new repository and push your theme there.
    • Fork and clone this repository
    • Add your theme in package.json
    • Add your theme name in copy-themes.sh
    • Push your changes to fork and deploy.

Compared to that, the method I am suggesting has fewer moving points:
* Fork and clone this repository
* Copy over your theme to content/themes/
* Push your changes and deploy
After all this repository aims to reduce work from the user, we want the work to be minimal.

  1. While I can see your method work great for open source themes (the fact that it will fetch the latest theme post-installation), it doesn't work at all if a user wishes to install a paid theme. The user won't want to host their theme on a public GitHub repository and currently, there is no way to fetch it from a private one.

  2. I can clearly see why your method makes sense for open source themes. We don't have to bloat the repository with extra stuff. But if the user wishes to add one extra theme, copying over shouldn't be a problem.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use private packages/verdaccio and such to get around "non-open-source" themes. My main concern is that it will clutter up your repository with the actual themes checked into the codebase, not version-tagged or anything like that. Good luck updating your themes, because you'll still need to do the "way too complex" steps of pinning your theme in package.json and in the script. It's LITERALLY two lines.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the theme should be equal work. It only involves copying the updated theme and redeploying. That is equivalent if to editing the package.json and redeploying.

You can use private packages/verdaccio and such to get around "non-open-source" themes

I don't think this is a good solution.

Copy link
Author

@aayushdutt aayushdutt May 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaneJeon I think your method is the best when we are dealing with open-source themes which are already there on GitHub.
I can't find a better way to easily add (private) themes without managing multiple repositories and editing a bunch of files. Copying over and pushing is the quickest and easiest to go with no major downsides.

If you think this change won't improve the project, feel free to close this PR.
A quick note though, it will be really good to value an open-source contributor's efforts and time a little more. Having an open-minded and polite discussion always helps.

3. Push the change
4. Deploy to Heroku by clicking the button below

[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy)
JaneJeon marked this conversation as resolved.
Show resolved Hide resolved

You can find your new theme in the Ghost admin panel.

In case you wish to **update existing Heroku deployment** with a new theme, follow steps 1 - 3 and then do the steps mentioned in [Updating the source code](https://github.com/vyronarediano/slp-on-heroku#updating-source-code)

## Updating source code

Optionally after deployment, to push Ghost upgrades or work with source code, clone this repo (or a fork) and connect it with the Heroku app:
Optionally after deployment, to push Ghost upgrades or work with source code, fork this repository and make required changes.

**Connect the app to your Heroku deployment:**
```bash
git clone https://github.com/snathjr/ghost-on-heroku
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't really be removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see a point in cloning this repository, making a change in it and not tracking that change. The correct way to change the source code should always be to fork

cd ghost-on-heroku

heroku git:remote -a YOURAPPNAME
heroku info
```

Then you can push commits to the Heroku app, triggering new deployments:

JaneJeon marked this conversation as resolved.
Show resolved Hide resolved
```bash
git add .
git commit -m "Important changes"
Expand Down