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

add a /wms route that returns json #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yanick
Copy link
Contributor

@yanick yanick commented Jul 17, 2020

This is a first, half-hapharded stab at it, but maybe it can be useful to start on #8

This is a first, half-hapharded stab at
it, but maybe it can be useful to start
on jmacdotorg#8
@jmacdotorg
Copy link
Owner

I reacted to this pull request with both joy and horror -- the latter entirely due to witnessing someone else trying to use Web::Mention's very sub-par JSON methods, dashed off two years ago for a single, now-obsolete use-case, and never revised since then.

I could not let that stand, and so we have this entirely separate pull request: jmacdotorg/webmention-perl#10 Could you please check it out at your convenience? It adds new JSON serialization/deserialization methods to Web::Mention.

If this seems like a good improvement to Web::Mention, then I'd like to change this PR's implementation to the following:

  • Letting Whim::Mention subclass TO_JSON so that it sticks the photo-hash value into the returned data structure
  • Invoking $wm->as_json instead of $wm->TO_JSON, since I'd rather let that be the public interface for serialization

And this should obviate the need to manually define all the author info, too.

@yanick
Copy link
Contributor Author

yanick commented Jul 18, 2020

I reacted to this pull request with both joy and horror -- the latter entirely due to witnessing someone else trying to use Web::Mention's very sub-par JSON methods, dashed off two years ago for a single, now-obsolete use-case, and never revised since then.

Keep a skeleton in your closet long enough, someone is bound to find it and want to play the xylophone with it. ^.^

I could not let that stand, and so we have this entirely separate pull request: jmacdotorg/webmention-perl#10 Could you please check it out at your convenience? It adds new JSON serialization/deserialization methods to Web::Mention.

If this seems like a good improvement to Web::Mention, then I'd like to change this PR's implementation to the following:

* Letting Whim::Mention subclass `TO_JSON` so that it sticks the photo-hash value into the returned data structure

* Invoking `$wm->as_json` instead of `$wm->TO_JSON`, since I'd rather let that be the public interface for serialization

And this should obviate the need to manually define all the author info, too.

It all get ❤️ from me. Wunderful. And thanks!

@jmacdotorg
Copy link
Owner

I've released version 0.720 of Web::Mention, and encourage you to make use of that towards the two changes I suggested yesterday.

I don't want to overwhelm you with lots of change requests at one, so let's take this in steps! Be assured that your contribution yesterday opened my eyes to how this is a simpler enhancement than I had earlier feared, so I'm excited to see it happen.

@yanick
Copy link
Contributor Author

yanick commented Jul 19, 2020

I've released version 0.720 of Web::Mention, and encourage you to make use of that towards the two changes I suggested yesterday.

Woo! Awesome!

I don't want to overwhelm you with lots of change requests at one, so let's take this in steps! Be assured that your contribution yesterday opened my eyes to how this is a simpler enhancement than I had earlier feared, so I'm excited to see it happen.

Heh heh. I am hard to overwhelm, so don't worry too much on my behalf. Still, thanks for the gentle approach. :-)

And, inversely, please be comfortable to deal with my PRs at your pace, and don't hesitate to ask for tweaks, or just say "thanks, but no thanks" to any of them. I typically like to send more than less, as even if they are not ultimately merged in, they can still trigger discussions and unexpected considerations. Which they seem to be doing here fabulously. :-)

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