Skip to content
This repository has been archived by the owner on Jul 5, 2021. It is now read-only.

add sarah theme #349

Closed
wants to merge 7 commits into from
Closed

add sarah theme #349

wants to merge 7 commits into from

Conversation

avelino
Copy link

@avelino avelino commented Apr 7, 2018

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2018

CLA assistant check
All committers have signed the CLA.

@digitalcraftsman
Copy link
Member

Hello @avelino,

thank you for sharing this theme with the Hugo community. 👍

To take the last mile please make sure that ìmages/screenshot.png has the right dimensions. You can find them in the README.

I noticed that the blog section on the homepage is empty. You might add a few (dummy) blog posts. Otherwise the blog menu link points to a 404 page.

Furthermore, the images for the projects that are set in the config file aren't present in the repo and hence not shown in the theme demo.

Tip: you can view your theme with the exampleSite if you cd into exampleSite and run hugo server -t ../..

avelino added a commit to avelino/hugo-theme-sarah that referenced this pull request Apr 8, 2018
@avelino
Copy link
Author

avelino commented Apr 8, 2018

@digitalcraftsman fixed screenshot

run hugo server -t ../.. and not crash home page

λ ~/src/github.com/avelino/hugo-theme-sarah/exampleSite/ master* hugo server -t ../.. -v --disableFastRender --bind 0.0.0.0 -b http://dev.avelino.xxx:1313/
INFO 2018/04/08 14:32:48 Using config file: /home/avelino/src/github.com/avelino/hugo-theme-sarah/exampleSite/config.toml
Building sites … INFO 2018/04/08 14:32:48 syncing static files to /
WARN 2018/04/08 14:32:48 No translation bundle found for default language "en"
WARN 2018/04/08 14:32:48 Translation func for language en not found, use default.
WARN 2018/04/08 14:32:48 i18n not initialized, check that you have language file (in i18n) that matches the site language or the default language.

                   | EN
+------------------+-----+
  Pages            |   7
  Paginator pages  |   0
  Non-page files   |   0
  Static files     | 149
  Processed images |   0
  Aliases          |   0
  Sitemaps         |   1
  Cleaned          |   0

Total in 11 ms
WARN 2018/04/08 14:32:48 Skip dataDir: lstat /home/avelino/src/github.com/avelino/hugo-theme-sarah/exampleSite/data: no such file or directory
WARN 2018/04/08 14:32:48 Skip i18nDir: lstat /home/avelino/src/github.com/avelino/hugo-theme-sarah/exampleSite/i18n: no such file or directory
WARN 2018/04/08 14:32:48 Skip layoutDir: lstat /home/avelino/src/github.com/avelino/hugo-theme-sarah/exampleSite/layouts: no such file or directory
Watching for changes in /home/avelino/src/github.com/avelino/hugo-theme-sarah/exampleSite/{content,static,..}
Serving pages from memory
Web Server is available at http://dev.avelino.xxx:1313/ (bind address 0.0.0.0)
Press Ctrl+C to stop

screenshot 2018-04-08 15 33 09

@digitalcraftsman
Copy link
Member

digitalcraftsman commented Apr 9, 2018

run hugo server -t ../.. and not crash home page

Do you've uncommitted files like the project images in your repo?
In config.toml you're linking the local image awesomego.png but isn't pushed to this repo..


When linking assets like images I would suggest to use {{ "path/to/asset.png" | absURL }}. Currently, the project are linked relative to the root directory of the server. But the theme demos are hosted in a subdirectory. Hence the browser tries to fetch the image from http://example.com/path/to/asset.png instead of http://example.com/sarah--theme-demo/path/to/asset.png. absURL ensures that the images are relative to the base url even if you're linking the root directory.

avelino added a commit to avelino/hugo-theme-sarah that referenced this pull request May 21, 2018
* https://github.com/gohugoio/hugoThemes: (32 commits)
  Add cpasule theme
  Update themes
  Add reveal-hugo theme
  Add er theme
  script: Match whole words when checking blacklist and noDemo
  Bump Hugo version to 0.40.3
  Add alpha-church theme
  Add shopping-product-catalogue-simple theme
  Update themes
  Add seminyak theme
  Update themes
  build: Escape left brace in Perl regex
  Bump Hugo version to 0.40.2
  Update themes
  Update themes
  Add sublime theme
  Update themes
  Add startpage theme
  Bump Hugo version to 0.40
  Add material-blog theme
  ...
@avelino
Copy link
Author

avelino commented May 21, 2018

@digitalcraftsman sorry slow reply

@avelino
Copy link
Author

avelino commented Jun 5, 2018

Any recommendation for improvement to be accepted?

@digitalcraftsman
Copy link
Member

Hello @avelino

sorry slow reply

I wasn't sure if there are any new updates. Could you give me a status update?

@avelino
Copy link
Author

avelino commented Jun 8, 2018

@digitalcraftsman 100% Updated

@avelino
Copy link
Author

avelino commented Jun 11, 2018

Pls accept this PR, I've updated gitsubmodules several times

@ghost
Copy link

ghost commented Jun 29, 2018

@avelino If I may speak on behalf of @digitalcraftsman I'm sure he wants nothing more than to assist you in getting this merged. There's a nice backlog of themes right now and I'm certain as soon as he has bandwidth he'll be tackling this for you. Thank you for your patience. 😃

@avelino
Copy link
Author

avelino commented Jun 29, 2018

@JHabdas no problem, I'm just asking to do the merge because I'm doing better constantly with the master to keep up to date

@digitalcraftsman
Copy link
Member

digitalcraftsman commented Jul 4, 2018

Hello everyone,

@avelino you're theme works as it is. However, the demo is still missing the images for the projects defined in config.toml and the menu item "blog" links to a non-existing page Look at my comment from April 9th for more information.

Those points doesn't make the theme demo unusable, but it might not make it look complete. It's up to you whether you would like to get the theme merged in it's current state or not.

Regarding updating the pull request: I should have said this earlier, but there's no need to rebase against master with each new commit. I can cherry-pick your commit(s) that adds your theme as submodule in this case.

Overall, I prefer to have theme submissions in form of an issue with a link to a repo instead of a pull request. This way I can switch quicker between submitted themes and don't have to pull the branch for the pull request first. Furthermore, there's no need for the submitter to update the pull request over and over again. This way it's less hassle for everyone.

@bep
Copy link
Member

bep commented Jul 27, 2018

This repository does not do Pull Requests. Open an issue with a link to your repository when you have something that you think is ready to merge.

@bep bep closed this Jul 27, 2018
@avelino avelino deleted the added_sarah_theme branch April 25, 2021 17:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants