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

Use composer.json and autoload #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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'

@@ -19,35 +19,31 @@
* function: WebPush Headers class =>WebPushConfig(headers)
* =>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'

@@ -19,33 +19,30 @@
* function: WebPushHmsOptions class =>WebPushConfig(hms_options)
* =>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'

@@ -19,93 +19,114 @@
* function: WebPushNotification class =>WebPushConfig(notification)
* =>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