hashobject / perun

Programmable static site generator built with Clojure and Boot (HELP NEEDED!)
https://perun.io
Eclipse Public License 1.0
351 stars 38 forks source link

Fix: RSS and Atom Feeds #251

Closed munen closed 3 years ago

munen commented 3 years ago

Both Atom and RSS feeds had some issues. This PR is not a perfect solution, but it improves the situation quite a bit.

Changes for Atom:

image

image

The issue was not trivial to fix, because the docs of clojure.data.xml state namespaces as a new feature. And this new syntax would not work on the old version. Finally, I realized that upgrading to the current version would give me access to said syntax. And since atom.clj is not specifying any namespace, now none is rendered and will make the Atom feed valid.

Unfortunately, the tests are now breaking. They run very slow for me and I don't know how to debug them. Hence, I left them broken (apologies). Simply reverting the clojure.data.xml version will make the tests run again, but then the feed will not be valid.

If someone teaches me how to run perun with Cider so that I can do some debugging or run just one test, I'd be happy to give that a shot at some point.

Changes for RSS:

Firstly, they are not nearly as huge as the Atom issue(;


As a result, both RSS and Atom feeds for 200ok.ch are valid now:

Both still have recommondations that could be addressed, but at least they are valid and work for all tested feed readers - including the ones that silently didn't work for one reason or another(;

allentiak commented 3 years ago

Thanks for the feature, @munen !

That being said, I think merging it with a broken test would be a pity... If you want, we could meet to do some pair programming in the upcoming weeks, so we give it a look? Maybe if we work together we can make it work... I haven't worked on this code for a long time...

munen commented 3 years ago

Thank you for inviting me to this repository @allentiak :pray:

That being said, I think merging it with a broken test would be a pity...

I agree. It's not even just a broken test, that I probably could have fixed. It's crashing with a stack trace. Hence, debugging would be benefitial.

For our site (200ok.ch), I've received mails from readers that they couldn't read the blog via Atom (which we link to in the header). My main priority was to get that to work and then to see if there's interest upstream - which I am very happy to see(;

If you want, we could meet to do some pair programming in the upcoming weeks, so we give it a look? Maybe if we work together we can make it work...

That's a very kind offer which I'd happily take you up on :pray:

Are you currently in UTC-3? If so, then we are five hours apart (I'm at UTC+2). I'm currently doing mostly product work, so I don't have too many strict appointments. For example, I could do today or tomorrow in our afternoon, so your morning.

I haven't worked on this code for a long time...

No worries. I know that you've reached out for help on this project. I haven't stepped up for two reasons mainly: There's lots of other stuff (I'm not the only one here, though), and I'm not what I personally would call familiar with boot or the Perun code base. Having said that, we've invested quite a bit into building 200ok.ch based on Perun. So I do have a high motivation to fix bugs and keep the project going, because otherwise at some point we would have to migrate away. And rebuilding doesn't always/often make things better(;

allentiak commented 3 years ago

@munen Thanks for your message! I've replied you privately.

munen commented 3 years ago

Thanks to an answer of Alex Miller himself on SO, the tests now run and we have a feed without an auto-generated prefix.

However, the feed arguably has become become valid, but moved further into the invalid:

image

munen commented 3 years ago

I have updated the question on SO and answered Mr. Alex Miller: https://stackoverflow.com/questions/64611482/how-to-generate-a-xml-without-a-namespace-with-the-latest-version-of-org-clojure

allentiak commented 3 years ago

I saw you got some interesting replies to your question ;-)

munen commented 3 years ago

I saw you got some interesting replies to your question ;-)

Yes, so :crossed_fingers: that it'll get resolved there, too(;

allentiak commented 3 years ago

we have a feed without an auto-generated prefix.

@munen That certainly is progress :-)

However, the feed [...] moved further into the invalid

@munen I saw the error messages, but... What do you mean by this, exactly? Is this the validator bug you mentioned in your SO question?

munen commented 3 years ago

@munen I saw the error messages, but... What do you mean by this, exactly? Is this the validator bug you mentioned in your SO question?

I wouldn't call it a 'validator bug' which implies in a bug in the validator, but yes I mean the update that I've added to the SO question where I go into further detail. With the proposed fix from Alex Miller, the feed actually has more errors, even though the first error went away.

allentiak commented 3 years ago

I see. Thanks for the explanation, @munen!

munen commented 3 years ago

Tests are green now:

Ran 6 tests, 94 assertions, 0 failures, 0 errors.

Also, both RSS and Atom feeds are valid.

This PR is ready to merge.

allentiak commented 3 years ago

@munen Thank you very much for your hard work!

Merged :-)