peter279k commented 5 years ago

Changed log

It's related to issye #1.
Initialize L10N class test.
Integrate Travis CI build.

peter279k commented 5 years ago

You are right.

How about using if ($In !== '') to check whether the $In is not a empty string?

Caleb Mazalevskis notifications@github.com 於 2019年3月30日週六下午7:23 寫道：

@Maikuolan commented on this pull request.

In src/YAML.php https://github.com/Maikuolan/Common/pull/3#discussion_r270622457:

@@ -38,7 +38,7 @@ class YAML */ public function __construct($In = '') {

if ($In) {

if (isset($In)) {

$In should already always be set anyway, due to that it's one of method's parameters. In this case, simply checking truthiness would be better, because when instantiating the object, if the parameter is omitted (which is okay to do, because it has a default value; an empty string), $In will still exist (i.e., will be set), but will be an empty string (so, will evaluate as false).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Maikuolan/Common/pull/3#pullrequestreview-220817613, or mute the thread https://github.com/notifications/unsubscribe-auth/AImpM7_VYlHxnwpwKtPMq2VwlijnYF_9ks5vb0k-gaJpZM4cTAO5 .

Maikuolan commented 5 years ago

(Just as a quick reply, for now: Keep the PR open; I'll look at this more closely and reply again soon. I hadn't replied since the last few days due to being busy with some other coding work. Anyway, the help is definitely appreciated, and thanks for the patience).

peter279k commented 5 years ago

Hi @Maikuolan, thank you for your reply.

I appreciate that you can review this PR at your available time :).

Maikuolan commented 5 years ago

I'll merge the PR now, to avoid too much delay at my side, and if there are any problems, we can fix those problems afterwards. I haven't yet looked super closely at the tests included in the PR, but at the same time, I'm also working new classes to be added to the repository, so it's good, I think, if I don't take too long to merge the PR.

And thanks again; I really appreciate the help. :-)

Maikuolan / Common

Initialize L10N test #3

Changed log

@Maikuolan commented on this pull request.