Skip to content

Conversation

@zmughal
Copy link
Contributor

@zmughal zmughal commented Jul 10, 2022

  • Use 2.x Elasticsearch client
  • Update API endpoint to POST favorite
  • Update search params for finding latest releases

@oalders oalders requested a review from mickeyn July 11, 2022 02:26
@mickeyn
Copy link
Contributor

mickeyn commented Jul 11, 2022

the examples as are in this branch give an error (the requests produce a 400 error), the code before the change is also not up to date, but let's make sure it all works perfectly before we merge it.
@zmughal can you please share the output you get from running this and the exact command you run the example with?

@mickeyn
Copy link
Contributor

mickeyn commented Jul 11, 2022

looks like my bad on the 400.
still the output of the examples doesn't look so good.
let's fix that.

Copy link
Contributor

@mickeyn mickeyn left a comment

Choose a reason for hiding this comment

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

these changes are ok.
as a follow up we should rework the outputs.

@mickeyn mickeyn merged commit 141bfa8 into metacpan:master Jul 11, 2022
@zmughal zmughal deleted the update-es branch July 11, 2022 21:41
@zmughal
Copy link
Contributor Author

zmughal commented Jul 11, 2022

I don't know if this helps, but Data::Printer has support for pretty-printing various HTTP-related data under Data::Printer::Filter::Web. Might make it easier to follow.

@oalders
Copy link
Member

oalders commented Jul 12, 2022

Thanks @zmughal. I did not know about that filter. Are you saying we should use it in some of the examples?

@mickeyn
Copy link
Contributor

mickeyn commented Jul 12, 2022

Thanks @zmughal

I don't have a problem with the Data::Printer output, I was mostly referring to the mix of it with the tracing of the curl output which is not adding up to a nice output.

Maybe we should just turn off the tracing? I don't think it should be part of the output (maybe only turn it on by a special debugging param)

@zmughal
Copy link
Contributor Author

zmughal commented Jul 15, 2022

Maybe we should just turn off the tracing? I don't think it should be part of the output (maybe only turn it on by a special debugging param)

Just sent in PR #24 as an attempt at that.

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.

3 participants