Website: Drop minima theme, fix RSS feeds #641
Conversation
File
|
eip-review-bot
changed the title
|
Thank you for taking a look at this! I'll try to review this and merge it at some point. Is there a matching PR for EIPs? If not, don't worry about making one. I'll take care of it. I am (very slowly) working on moving to Zola for rendering EIPs/ERCs. I'm not much of an HTML expert, but I have a theme that replicates the look of the current website. If you're interested, I'd love some help over there too! |
|
I will need a slightly different fix for EIP repo. And I will do that. But I need some momentum on my PRs getting merged to continue on this. |
|
Hello hello. Can I please get a merge on this PR? |
| {%- feed_meta -%} | ||
| <link type="application/atom+xml" rel="alternate" href="{{ '/rss/all.xml' | relative_url }}" title="{{ site.title | escape }}" /> |
There was a problem hiding this comment.
I don't know Jekyll at all. Can you explain?
There was a problem hiding this comment.
The line that is being removed is a bunch of magic for generating RSS, which is not the RSS we want.
The line that is being added is an explicit RSS feed that makes sense for this repo.
Gemfile
Outdated
| @@ -10,10 +10,7 @@ source "https://rubygems.org" | |||
| # This is the default theme for new Jekyll sites. You may change this to anything you like. | |||
| gem "minima", "~> 2.0" | |||
There was a problem hiding this comment.
Yes, removing the line has no impact. Removed in a new commit.
To see my current review queue, take a look at EIPs Insight. |
|
A merge on this PR here directly increases visibility of our EIPs for reviewers (with RSS and email notification) thus decimating work in your queue. :-) |
|
I think this is a great direction. I am in favor of this PR to be merged. That said, is there a link we can try it? |
|
Sorry we didn't set up webpage builds here for PRs. But I did set up screenshots above, it works on responsive. And if there is something broken (even if I didn't break) you can see I'm quick to maintain things |
|
Here is what the page looks like: Screen.Recording.2024-11-15.at.15.20.51.movThe only visual change is in the header at top. This is demonstrated in the video by showing the header and then by showing another page which is not impacted. @xinbenlv is this what you are looking for to review it? Build instructions for this PR and for the project as normal: bundle install
bundle exec jekyll serve --verbose --livereload^ Those instructions should be in the readme. And I will propose that PR. But my PRs on this repo here are on hold until this PR is merged. |
The work for this PR is to fix the RSS feeds on the ERCs website.
The problem was that the Jekyll Minima theme interferes with any custom RSS feed. (We use a custom RSS feed because people care about our ERCs, not changes to our other pages.) And therefore it was necessary for me to remove the Minima theme (and bump gem deps) to achieve this.
In this process I took the minor liberty of using the Bootstrap 5 CSS (which we were already using) to renter the header and footer rather than recreating dozens of Minima rules that we were previously relying on (and again which were duplicative to Bootstrap 5).
I did NOT take the liberty of fixing the header or footer, or all kinds of other problems with the site. I would like to do so, but that is not part of this minimal PR.
Here is what the page looks like. So you can see the slight update in style which is a side effect of this PR. And you can see the now working RSS feeds. Also the metadata on the pages linking to the RSS is fixed (and not visible in the screenshots).