-
Notifications
You must be signed in to change notification settings - Fork 137
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
Support for file operations #103
Conversation
e8b13f2
to
4d4d66a
Compare
4d4d66a
to
922b768
Compare
examples/FileSharing.php
Outdated
|
||
// Listing files in the channel | ||
$channelFiles = $pubnub->listFiles()->channel($channelName)->sync(); | ||
if ($channelFiles->getCount() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about introducing:
$fileCount = $channelFiles->getCount();
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would improve readability
public function encryptMessage($message) | ||
{ | ||
$crypto = $this->pubnub->getCryptoSafe(); | ||
$message = PubNubUtil::writeValueAsString($message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about renaming to:
$messageString
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
|
||
protected function buildMessage() | ||
{ | ||
$message = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't $messageData
be a more descriptive name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
{ | ||
$params = []; | ||
foreach ($this->customParamMapping as $customParam => $requestParam) { | ||
if (isset($this->$customParam) && !empty($this->$customParam)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is issset
required here? What is a responsibility of empty
method check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isset
is a safeguard for accessing unintialized typed property and !empty
checks if the value is not empty (not null, 0
or empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I thought that empty
implicitly performs isset
because:
"The isset
function only checks if a variable is set and not NULL"
->fileId($this->fileUploadEnvelope->getFileId()) | ||
->fileName($this->fileName); | ||
|
||
if (isset($this->meta)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is duplicated in line 269, 272, 275 is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unintentional copypaste
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
1fb3ba8
to
43c51a1
Compare
{ | ||
$params = []; | ||
foreach ($this->customParamMapping as $customParam => $requestParam) { | ||
if (isset($this->$customParam) && !empty($this->$customParam)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I thought that empty
implicitly performs isset
because:
"The isset
function only checks if a variable is set and not NULL"
@pubnub-release-bot release |
🚀 Release successfully completed 🚀 |
feat: Added support for file sharing operations