-
Notifications
You must be signed in to change notification settings - Fork 4
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
For review: A developer's toolkit for OPML support #1
Comments
I downloaded the repo and played around with it - I like it! I do have a few comments:
I created a version of the client demo using a OPML booklist file (http://andysylvester.com/files/opmlclient/), it was easy to modify the example code/files. Overall - looks good! I am planning to use this for my Federated Bookshelves project. |
@andysylvester -- thanks for the quick feedback.
I'm glad you'll put this package to use in your application, that's the real test. |
Updates look good - thanks! |
The node side looks great, exactly how I'd imagine it would work. It might not be reasonable, but I'd love to see if there was a way to make a single library work as either client or server-side instead of having two separate implementations to manage. I'll do some research to see how isomorphic packages work. Either way, do you think some of the files in the client folder should actually be under examples? |
@andrewshell --
|
BTW, there is a problem with the way it works in Node. It really shouldn't appear to be asynchronous, because -- it's quite synchronous. The xml2js package made imho a poor design decision, it has been debated in their issues section long before I arrived on the scene. I pitched in my two cents, but they aren't budging. Simple proof that it doesn't need to be async -- JSON.parse is synchronous. Anyway, it's a nit, but I cared about this nit for this foundational code. |
@andrewshell -- if I had my druthers, this is what you'd work on next...
|
@andrewshell -- I implemented your suggestion of splitting off the example from the client code. Very clean and simplifying. Look for broken links which always come with these kinds of reorgs. ;-) |
First, I'll update instructions with the default folder and Next, I'll write a section about hosting an OPML file using the mirrors feature because that will include information about how to create an OPML file with http://littleoutliner.com/ and get the public URL. |
@andrewshell -- Sorry for the confusion -- I meant that the howto document should be an opml file served by pagepark. I think having the first file they create be a markdown file is the right choice. |
|
A new toolkit, in the form of an NPM package.
https://github.com/scripting/opmlpackage
I wanted to get a review before putting it out there, in case there are any glaring mistakes, omissions and typos. I understand of course it's a national holiday, this just happened to be when it was ready for review.
If you have any comments or questions or whatever, please add to this thread. Any time in the next few days would be helpful. Thanks! :-)
The text was updated successfully, but these errors were encountered: