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

A first draft of an Atom syndication format plugin. #43

Merged
merged 5 commits into from
Mar 5, 2023
Merged

A first draft of an Atom syndication format plugin. #43

merged 5 commits into from
Mar 5, 2023

Conversation

Tim-ats-d
Copy link
Contributor

@Tim-ats-d Tim-ats-d commented Feb 22, 2023

We discussed it here so here is a first try with several weaknesses:

  • I tried as much as possible to have an API similar to Yocaml.Rss but I didn't know if it was wise that the parameters that are URLs should be Uri.t or more naturally string.

  • In Atom.make, the data used to create generator parameter are hardcoded, they could be replaced by generating the yocaml .opam file with dune and using watermarks replaced by dune.

  • I don't know if the data such as authors must also be wrapped in their own data type to erase syndic traces

In addition, I fixed a small detail Format.printf "\"%s\"" str can be replaced by Format.printf "%S" str.

@Tim-ats-d
Copy link
Contributor Author

cc @xhtmlboi

@xhtmlboi xhtmlboi self-requested a review February 27, 2023 21:53
Copy link
Owner

@xhtmlboi xhtmlboi left a comment

Choose a reason for hiding this comment

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

I think what would be great is :

  • Have Rss support in yocaml_syndication
  • Have a hook that converts the metadata (Articles) to a feed, similar to how it is done in YOCaml.

Anyway, thank you very much for this contribution and sorry for the delay!

@Tim-ats-d
Copy link
Contributor Author

  • Have Rss support in yocaml_syndication

I would have liked to but I am stuck by the lack of function for output a Syndic.Rss2.channel to XML.

@xhtmlboi
Copy link
Owner

xhtmlboi commented Mar 3, 2023

  • Have Rss support in yocaml_syndication

I would have liked to but I am stuck by the lack of function for output a Syndic.Rss2.channel to XML.

You are right Cumulus/Syndic#75
I guess that we can merge! Thanks a lot for your work!

xhtmlboi
xhtmlboi previously approved these changes Mar 3, 2023
@xhtmlboi xhtmlboi dismissed their stale review March 3, 2023 07:34

It seems that the current branch does not typecheck.

Copy link
Owner

@xhtmlboi xhtmlboi left a comment

Choose a reason for hiding this comment

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

Just a small fix (and after I can merge)

@Tim-ats-d
Copy link
Contributor Author

Tim-ats-d commented Mar 3, 2023

Oops. I also added a link to the syndic library in the doc.

@Tim-ats-d
Copy link
Contributor Author

Fix #42 for posterity, I guess 😛

@xhtmlboi xhtmlboi merged commit 4abfdb0 into xhtmlboi:main Mar 5, 2023
@xhtmlboi
Copy link
Owner

xhtmlboi commented Mar 5, 2023

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants