Skip to content

Conversation

@trbsi
Copy link

@trbsi trbsi commented Oct 22, 2020

No description provided.

@trbsi trbsi force-pushed the composer-json-autoload branch from 813dff3 to 4f9e49c Compare October 22, 2020 12:11
'bi_tag',
'fast_app_target',
'biTag',
'fastAppTarget',
Copy link
Author

Choose a reason for hiding this comment

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

For the reviewer: Does this has to be "fast_app_target" or it can be "fastAppTarget"

I replaced "_" in every buildFields() method

@trbsi
Copy link
Author

trbsi commented Oct 26, 2020

@HMSPushKit @Mike-mei @rmibelgium Can anyone please review this PR?

@trbsi trbsi changed the title User composer.json and autoload Use composer.json and autoload Oct 26, 2020
@Mike-mei
Copy link
Contributor

Hello @trbsi
Thank you for PR, we'll look at your changes but there are different development processes in our team so we can't merge your PR directly, we will review your PR and reply to you asap. Thank you for your contribution 🍻

@trbsi
Copy link
Author

trbsi commented Oct 29, 2020

@Mike-mei
Hmm so basically setters and getters will be rejected, property camel-case will be rejected... What about composer.json?

Copy link
Contributor

@HMSPushKit HMSPushKit left a comment

Choose a reason for hiding this comment

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

change function name could be accepted or fileName,but the attribute name can't be changed.
the include file has been verify in linux and windows, not need to change.

@@ -0,0 +1,26 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

not used this php architecture since 3rd attribute

@@ -0,0 +1,19 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

not used 3rd attribute

"php": ">=7.0"
},
"platform-dev": []
}
Copy link
Contributor

Choose a reason for hiding this comment

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

BIN not allowd to use

@@ -0,0 +1,151 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

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

Example/test_sample_push_topic_msg.php has it,not need it

use push_admin\PushLogConfig;
namespace Huawei\Example\PushCommon;

use Huawei\PushNotifications\PushAdmin\AndroidConfigDeliveryPriority;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is “Huawei” Directory?

@@ -18,19 +18,14 @@
/**
* function: webpush message push control parameter, which is required for webpush msg=>PushMessage(webpush)
*/
namespace push_admin\push_msg\webpush;
Copy link
Contributor

Choose a reason for hiding this comment

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

include using has verify in linux and windows,not necessary to 'use'

* =>PushMessage(webpush)
*/
namespace push_admin\push_msg\webpush;
namespace Huawei\PushNotifications\PushAdmin\PushMessage\WebPush;
Copy link
Contributor

Choose a reason for hiding this comment

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

include using has verify in linux and windows,not necessary to 'use'

*/
namespace push_admin\push_msg\webpush;

namespace Huawei\PushNotifications\PushAdmin\PushMessage\WebPush;
Copy link
Contributor

Choose a reason for hiding this comment

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

include using has verify in linux and windows,not necessary to 'use'

* =>PushMessage(webpush)
*/
namespace push_admin\push_msg\webpush;
namespace Huawei\PushNotifications\PushAdmin\PushMessage\WebPush;
Copy link
Contributor

Choose a reason for hiding this comment

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

include using has verify in linux and windows,not necessary to 'use'


private $title;

private $icon;
Copy link
Contributor

Choose a reason for hiding this comment

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

include using has verify in linux and windows,not necessary to 'use'

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