manavo commented 4 years ago

Hey @ppshobi, thanks for the awesome library!

Found an edge case which was failing, so here's a failing test with a fix included too.

Scrutinizer seems to be failing, but it's failing to actually run phpunit. I don't think that's related to this PR though, didn't make any changes that would affect that (I don't think at least)!

codecov[bot] commented 4 years ago

Codecov Report

Merging #18 into master will increase coverage by 0.04%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master      #18      +/-   ##
============================================
+ Coverage     95.21%   95.25%   +0.04%     
- Complexity       83       84       +1     
============================================
  Files            22       22              
  Lines           230      232       +2     
============================================
+ Hits            219      221       +2     
  Misses           11       11

Impacted Files	Coverage Δ	Complexity Δ
src/Commands/Command.php	`100.00% <100.00%> (ø)`	`3.00 <1.00> (+1.00)`
src/Commands/Ingest/PopCommand.php	`100.00% <100.00%> (ø)`	`1.00 <0.00> (ø)`
src/Commands/Ingest/PushCommand.php	`100.00% <100.00%> (ø)`	`2.00 <0.00> (ø)`
src/Commands/Search/QueryCommand.php	`100.00% <100.00%> (ø)`	`4.00 <0.00> (ø)`
src/Commands/Search/SuggestCommand.php	`100.00% <100.00%> (ø)`	`3.00 <0.00> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1b4ddf4...bb0ff02. Read the comment docs.

manavo commented 4 years ago

Hey @ppshobi, thanks for the awesome library!

Found an edge case which was failing, so here's a failing test with a fix included too.

Scrutinizer seems to be failing, but it's failing to actually run phpunit. I don't think that's related to this PR though, didn't make any changes that would affect that (I don't think at least)!

ppshobi commented 4 years ago

Wow, nice. Thank you for your finding and PR. And Indeed, scrutinizer is failing due to an unrelated issue. When I installed scrutinizer first, it didn't have the docker support to run sonic, So I just let fail scrutinizer, since we already have Travis for the test runs. Scrutinizer is solely responsible for code quality.

ppshobi commented 4 years ago

just tagged a new minor version release, should be avialable in packagist soon

manavo commented 4 years ago

Nice, thanks for the quick reply and merge!

The travis and docker setup is really cool for running real tests, I’ll have to keep that in mind! 🙂

I was wondering if I could trouble you for a quick thing? Since hacktoberfest is going on anyway now, would you be able to add the hacktoberfest-accepted label to this PR?

Thanks!

ppshobi commented 4 years ago

Sure thing, why not. I will add it tomorrow as the first thing.

On Thu, 15 Oct 2020, 2:25 am Philip Manavopoulos, notifications@github.com wrote:

Nice, thanks for the quick reply and merge!

The travis and docker setup is really cool for running real tests, I’ll have to keep that in mind! 🙂

I was wondering if I could trouble you for a quick thing? Since hacktoberfest is going on anyway now, would you be able to add the ‘ hacktoberfest-accepted’ label to this PR?

Thanks!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ppshobi/psonic/pull/18#issuecomment-708712506, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBEEHZ2ANNHSO2CAXM5Z5TSKYXPNANCNFSM4SRBPSNQ .

manavo commented 4 years ago

Thanks 🙂

ppshobi commented 4 years ago

@manavo can you check if the labeling is fine?

manavo commented 4 years ago

That’s perfect, thanks!

ppshobi / psonic

Fix for handling quotes in the text itself when pushing #18

Codecov Report