djangoproject.com icon indicating copy to clipboard operation
djangoproject.com copied to clipboard

Add support for markdown blog posts

Open bmispelon opened this issue 1 year ago • 8 comments

I picked the first result on https://pypi.org/search/?q=markdown. Happy to switch to something else if someone has opinions ™️

bmispelon avatar Oct 17 '24 20:10 bmispelon

👏 👏 👏 👏 👏 👏

jefftriplett avatar Oct 17 '24 22:10 jefftriplett

@bmispelon Wow! You're speedy 😃

You asked for opinions, though, and that was your first mistake 😆😆😆


Seriously, though, I had added some comments on #1501 about maybe using a package that would allow markdown formatting and inclusion of images in posts, potentially knocking out two or more Issues in a single PR. But nobody commented so I didn't move forward with a PR.

https://github.com/django/djangoproject.com/issues/1501#issuecomment-2379674510

I haven't looked in any depth at your PR yet, so if it already achieves including images, please let me know and I'll quiet down.

jacklinke avatar Oct 18 '24 03:10 jacklinke

I picked the first result on https://pypi.org/search/?q=markdown. Happy to switch to something else if someone has opinions ™️

My only concern with the library you chose is the resistance to contribution I've felt in the past from the project's maintainers.

That package is a dependency of Pelican, which I use for my blog, and when I tried to submit issues or PRs, I didn't feel in the open and collaborative context of Django, so I stopped trying.

I've had the same feeling with a few other Python packages (e.g. flake8) that I've personally stopped using and have since been superseded by others as the de facto standard (e.g. ruff).

I suspect the same will happen with this package at some point, and I'd try to investigate a bit more than "the first package in PyPi results" before we commit our code to it.

I can't suggest an alternative package at the moment, but I'll try to investigate.

Would it be a good idea to make the dependency generalized enough that switching markdown conversion packages would be painless in the future?

pauloxnet avatar Oct 23 '24 01:10 pauloxnet

OMG!

thibaudcolas avatar Oct 23 '24 18:10 thibaudcolas

Thanks @thibaudcolas for the thorough review. The writer change was an oversight on my part (I had played around with swapping writers and didn't noticed I kept it in the final PR). I think it's better to keep the html writer for now. Otherwise existing rst blog entries migh risk being rendered differently the next time they're saved.

bmispelon avatar Oct 23 '24 20:10 bmispelon

I haven't looked in any depth at your PR yet, so if it already achieves including images, please let me know and I'll quiet down.

Sorry, I had missed your original comment on https://github.com/django/djangoproject.com/issues/1501#issuecomment-2379674510 and didn't know about it when I wrote this PR.

My PR doesn't have image upload support, but I'm still keen to move forward with it. My reasoning is that this change is quite small, and because we're using Markdown for rendering it means we can switch to using django-markdownx later down the road for fancier features (previews, images, ...).

What do you think?

bmispelon avatar Oct 23 '24 20:10 bmispelon

I suspect the same will happen with this package at some point, and I'd try to investigate a bit more than "the first package in PyPi results" before we commit our code to it.

In truth, I didn't just pick Markdown because it was the first result on PyPI. The package also seems widely used, and is actively being maintained based on the release history.

Would it be a good idea to make the dependency generalized enough that switching markdown conversion packages would be painless in the future?

My personal preference would be to ship the solution we have now: I believe it's good enough, it's a net positive, and it solves needs that people have right now. Engineering a more complex system based on the hypothetical need of having to switch dependencies doesn't seem worth it to me right now, I would rather tackle the problem when it presents itself.

The current solution is flexible enough in that we can always add more renderers to ContentFormat (including a different markdown variant that would use a different library if it comes to that).

bmispelon avatar Oct 23 '24 21:10 bmispelon

Would it be a good idea to make the dependency generalized enough that switching markdown conversion packages would be painless in the future?

My personal preference would be to ship the solution we have now: I believe it's good enough, it's a net positive, and it solves needs that people have right now. Engineering a more complex system based on the hypothetical need of having to switch dependencies doesn't seem worth it to me right now, I would rather tackle the problem when it presents itself.

The current solution is flexible enough in that we can always add more renderers to ContentFormat (including a different markdown variant that would use a different library if it comes to that).

I just wanted to share my bad experience with the package maintainers, but I didn't want to block the PR. If you think it's better to proceed with this dependency, go ahead, I hope we won't have any problems in the future.

pauloxnet avatar Oct 23 '24 21:10 pauloxnet

Yes please! Images would be neat but for now just an extra, better formatting option is already a big win.

thibaudcolas avatar Oct 24 '24 08:10 thibaudcolas