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 Mail.php #5495

Closed
wants to merge 1 commit into from
Closed

Update Mail.php #5495

wants to merge 1 commit into from

Conversation

pechtelt1
Copy link

@pechtelt1 pechtelt1 commented May 3, 2023

What does this PR do?

Fix the issue

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@@ -118,7 +118,7 @@ public function getBody(): string
*/
public function setName(string $name): self
{
$this->name = $name;
$this->name = $name ?? '';
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work because according to the typing on the function, $name is a string and will never be null

@stnguyen90
Copy link
Contributor

Please also provide some screenshot as a test that the change fixes the problem.

@pechtelt1
Copy link
Author

@stnguyen90 thank you very much! The thing is that I don't have much experience here yet and am just starting this and I thought of just joining open source projects to learn but don't know exactly how to run this locally to test everything properly.

Apologies but as soon as I know more I am happy to help on other issues!

@stnguyen90
Copy link
Contributor

@pechtelt1 you would have to start Appwrite and make the necessary API calls to trigger the endpoints. The contributing guide and docs should help you, but do you no longer want to work on this?

@pechtelt1
Copy link
Author

@stnguyen90 i will try, thanks!

@pechtelt1
Copy link
Author

It wont work for me... i hope someone else will able to fix it!

@stnguyen90
Copy link
Contributor

@pechtelt1, if you cannot make progress on this, can I close it and have someone else work on it?

@pechtelt1
Copy link
Author

@stnguyen90 yes thats fine!

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.

I've been experiencing an issue with the login page on the application
2 participants