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

limchengyang/add_callback_log_appid #48

Conversation

Projects
None yet
6 participants
@limchengyang
Copy link
Contributor

commented May 6, 2019

No description provided.

Nobody User and others added some commits May 6, 2019

Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User
Nobody User

@omar-binary omar-binary force-pushed the limchengyang:limchengyang/add_callback_log_appid branch from d78f298 to f032731 May 7, 2019

@codecov-io

This comment has been minimized.

Copy link

commented May 7, 2019

Codecov Report

Merging #48 into master will increase coverage by 0.67%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   73.97%   74.65%   +0.67%     
==========================================
  Files           7        7              
  Lines         365      359       -6     
  Branches       60       59       -1     
==========================================
- Hits          270      268       -2     
+ Misses         55       52       -3     
+ Partials       40       39       -1
Impacted Files Coverage Δ
lib/Mojo/WebSocketProxy/Dispatcher.pm 82.69% <57.14%> (+1.38%) ⬆️
lib/Mojo/WebSocketProxy/Parser.pm 41.66% <0%> (-1.58%) ⬇️
lib/Mojo/WebSocketProxy/Backend.pm 75.43% <0%> (+1.3%) ⬆️
lib/Mojolicious/Plugin/WebSocketProxy.pm 77.77% <0%> (+1.69%) ⬆️

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 6556b67...a66fe92. Read the comment docs.

Show resolved Hide resolved README.md Outdated
Show resolved Hide resolved README.md Outdated

Nobody User and others added some commits May 7, 2019

Nobody User
Merge branch 'limchengyang/add_callback_log_appid' of github.com:limc…
…hengyang/perl-Mojo-WebSocketProxy into limchengyang/add_callback_log_appid
Nobody User
return;
};

my $normalized_msg = eval { Unicode::Normalize::NFC($decoded) } or do {
stats_inc("websocket_proxy.unicode_normalisation.failure", {tags => ['error_code:1007']});
$c->finish(1007 => 'Malformed Unicode data');
$config->{on_error}({

This comment has been minimized.

Copy link
@binarytom

binarytom May 7, 2019

Contributor

Can you use ->emit instead? Mojolicious already has some basic event handling built in.

See https://github.com/binary-com/perl-Mojo-WebSocketProxy/blob/master/lib/Mojo/WebSocketProxy/Parser.pm#L52 for an example.

This comment has been minimized.

Copy link
@limchengyang

limchengyang May 8, 2019

Author Contributor

changes are made based on the comment

Changes Outdated
@@ -2,6 +2,12 @@ Revision history for Perl module Mojo::WebSocketProxy

{{$NEXT}}

0.12 2019-05-06 16:57:54+08:00 Asia/Minila

This comment has been minimized.

Copy link
@zakame

zakame May 8, 2019

Contributor

With this line and version generated, has the resulting dist been uploaded to PAUSE already?

This comment has been minimized.

Copy link
@zakame

zakame May 8, 2019

Contributor

Also I don't see any corresponding Makefile.PL changes

This comment has been minimized.

Copy link
@limchengyang

limchengyang May 8, 2019

Author Contributor

our bad, first time working on public module, didnt know that we should upgrade the versioning in another PR with dzil release. We somehow did this manually, removing it right now 😅

Nobody User and others added some commits May 8, 2019

@raunakkathuria raunakkathuria merged commit 23542c2 into binary-com:master May 15, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.