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

Update mongolid version 3 #21

Merged
merged 11 commits into from
Sep 15, 2022
Merged

Conversation

djonasm
Copy link
Contributor

@djonasm djonasm commented Feb 21, 2022

No description provided.

@djonasm djonasm self-assigned this Feb 21, 2022
@djonasm djonasm force-pushed the chore/update-mongolid-version-3 branch from 292032d to 5d9ecd4 Compare March 28, 2022 13:58
@djonasm djonasm force-pushed the chore/update-mongolid-version-3 branch from 5d9ecd4 to 7c050fb Compare March 28, 2022 13:58
@ezandonai ezandonai marked this pull request as ready for review May 4, 2022 12:15
*/
public function setSecretAttribute($value)
public function setSecretAttribute($value): string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does Mongolid use the return of a setter to fill a field?

$client->fill([
'secret' => $client->secret,
], true);
$client->setSecretAttribute($client->secret);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using fill is more intuitive than calling setter directly, and works with the syntax $client->secret = 'some-secret';. I would use fill.

But, if you don't want to change it, you'll have to fix the ClientCommand and look for other necessary changes.

@diegofelix diegofelix dismissed orlandocavassani’s stale review July 15, 2022 18:04

Orlando is on vacation.

@ezandonai ezandonai merged commit 9b59242 into master Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants