aspnet / live.asp.net

Code for live.asp.net, which hosts the ASP.NET Community Stand-up
https://live.asp.net/
MIT License
289 stars 114 forks source link

Add a `/feed` endpoint that returns an ATOM feed. Fixes #99. #105

Closed hashhar closed 4 years ago

hishamco commented 7 years ago

@hashhar I think you missed the calling part to access this from UI

hashhar commented 7 years ago

@hishamco Oops. I'll add a link. Should I add it just to the Index view or the shared razor files (the header or footer maybe?)

hishamco commented 7 years ago

IMHO adding the feed links in the footer is the best option 😄

hashhar commented 7 years ago

I did just that. What about using POCO? I'll try and use it and push a commit when it works.

hashhar commented 7 years ago

Only the POCO part remains now. Other than that everything works. I also support so that podcast apps can use the videos too.

hashhar commented 7 years ago

@hishamco @campersau This is finished. Nothing more to do here. Should I squash the commits or leave them as is?

Also, the empty default constructors are there only for the XMLSerializer to work.

hashhar commented 7 years ago

Pushed new changes.

hashhar commented 7 years ago

@hishamco I think I can't reduce POCOs because any XML Element that acts as a container for other elements needs to have it's own class since the XmlRoot annotation can only be applied to classes, strutcs, enums or returns. Something like <a> in the example below will need its own class as far as I know.

<a>
    <b foo="foo"/>
    <c bar="bar"/>
</a>
hishamco commented 7 years ago

@hashhar did you check the link I posted before?!!

hashhar commented 7 years ago

@hishamco Yes I read it. The problem is that I have three extra POCOs due to <author>, <content> and <link> tags. This is the minimum due to ATOM specification. But yes, I can get rid of a lot of these annotations by changing the variable names. I didn't know that.

hishamco commented 7 years ago

I think you're right, my aim to remove unnecessary classes, but after I looked SyndicationItem there are Link and Person and Content classes, it would be nice if you check this and come up with a beauty piece of code 😄 thanks

hashhar commented 7 years ago

@hishamco Wow, that's so good. I didn't know there was a reference source. I'll definitely take a look. But it may take some time since I have an exam tomorrow and the day after that.

I think this effort can be expanded and later ported over to ASP.NET Core. 😄

hishamco commented 7 years ago

The reference source is a great source of .NET :), regarding the Syndication Stuff, many of the previous APIs planned to come to the new Core world in upcoming NET STANDARDS

hishamco commented 7 years ago

FYI if we go with the required elements in the Atom feed we are able to remove unnecessary POCOs, so the remaining will be AtomFeed and Entry

hashhar commented 7 years ago

I simplified it the best I could. I removed the extra annotations where I could because by default XmlElement names are the same as variable names. I also made the change to DateTime as you suggested and converted Link and Entry to links and entries to make sense because they are Collections.

hishamco commented 7 years ago

Please have a look to my last blog here I know it's an Arabic 😄 , at least you can look to the code snippets, this is the less amount code I come up with to generate an ATOM feed, hope you like it

hashhar commented 7 years ago

I squashed it all into a single commit. I don't think I can trim it down any more. Changing it to what you have in your blog will break two things.

  1. In the current form, the content contains a thumbnail.
  2. The media enclosure links allow feed readers to treat this as a podcast and will allow downloading the video.

Should I add doc comments to public functions?

hashhar commented 7 years ago

I just now tested with my own API key and it works without any issues. See the result.

Image

hishamco commented 7 years ago

I didn't mean that you should follow what I did, but what i'm doing is the least amount of code to generate the ATOM feed I will try to test the code in my end, and let you know if there's any further suggestion, but in general that's great stuff 👍