search

preaction / Statocles

Static website CMS
http://preaction.me/statocles
Other
84 stars 33 forks source link

Rewrite Error message Address in use #485 #487

Open jluis opened 8 years ago

jluis commented 8 years ago

"Can't create listen socket: Address already in use" is rewriten as "Can't create listen socket on port $port: Address already in use"

Signed-off-by: Jose Luis Perez Diez jluis@escomposlinux.org

preaction commented 8 years ago

Thanks! You'll probably want to use $daemon->listen instead to get the address that can't be listened to (see http://mojolicious.org/perldoc/Mojo/Server/Daemon#listen). That way if Mojolicious changes how it finds the listen address, we aren't caught trying to keep up.

There needs to be a test written. Starting up one daemon before trying to start a second should be enough to make the failure case happen.

Is this the only error message that could be thrown by $daemon->start? It might be better to use a regex to match $@ before customizing the error message.

jluis commented 8 years ago

Thanks for your pointers.

I did the pull request so early, because I had not noticed that pushed commits that reference an issue got a link in the issue thread, to get feedback.

I plan to look at Mojo source to see how and what messages throws when dies as I have noticed that this particular message has parts that depend on $ENV{LANG} .

I will also create a test for it on t/command/daemon.t

I suppose that it is OK to include in the pull request both files and all chain of commits, isn't it?

preaction commented 8 years ago

This seems like it's a single change, so it should likely be a single commit. So you can amend your current commit, and then force-push to update the pull request. Otherwise, I can squash the commits when I merge, either way.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 92.633% when pulling 335a5a09a67e3b428bd2fe1fdcbb14a1525286e5 on jluis:cpanpr into 29e2c92746e1ef293df13231b8742e7bf88bacea on preaction:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.03%) to 92.874% when pulling 372257e9bd8309443b057e4a45508c81f1f6054c on jluis:cpanpr into dc7d8e8b43cb7aa76454969f0762a543828e0b1d on preaction:master.

jluis commented 8 years ago

I messed the pull request a little as I don't use normally amended commits. i@preaction I will try to addres other isues now

preaction commented 8 years ago

No worries, we can clean up the commits later. Let me know if you've got any questions.