Skip to content

Update Mail.php - #5495

Closed
pechtelt1 wants to merge 1 commit into
appwrite:masterfrom
pechtelt1:fix-5220-better-error-messages
Closed

Update Mail.php#5495
pechtelt1 wants to merge 1 commit into
appwrite:masterfrom
pechtelt1:fix-5220-better-error-messages

Conversation

@pechtelt1

@pechtelt1pechtelt1 commented May 3, 2023•
edited
Loading

Copy link
Copy Markdown

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?

@pechtelt1pechtelt1 mentioned this pull request May 3, 2023
Closed
2 tasks
stnguyen90
stnguyen90 suggested changes May 3, 2023
View reviewed changes
Comment threadsrc/Appwrite/Event/Mail.php
public function setName(string $name): self
{
$this->name = $name;
$this->name = $name ?? '';

@stnguyen90stnguyen90May 3, 2023

Copy link
Copy Markdown
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

stnguyen90 commented May 3, 2023

Copy link
Copy Markdown
Contributor

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

@Rajexpo97Rajexpo97 linked an issue May 3, 2023 that may be closed by this pull request
I've been experiencing an issue with the login page on the application #5498
Closed
@pechtelt1

pechtelt1 commented May 31, 2023

Copy link
Copy Markdown
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

stnguyen90 commented Jun 15, 2023

Copy link
Copy Markdown
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

pechtelt1 commented Jun 16, 2023

Copy link
Copy Markdown
Author

@stnguyen90 i will try, thanks!

@pechtelt1

pechtelt1 commented Jun 23, 2023

Copy link
Copy Markdown
Author

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

@stnguyen90

stnguyen90 commented Jun 28, 2023

Copy link
Copy Markdown
Contributor

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

@pechtelt1

pechtelt1 commented Jun 29, 2023

Copy link
Copy Markdown
Author

@stnguyen90 yes thats fine!

@pechtelt1pechtelt1 closed this by deleting the head repository Jun 29, 2023
@stnguyen90stnguyen90 mentioned this pull request Jul 22, 2023
Merged
This was referenced Jul 25, 2023
Closed
Closed
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@stnguyen90stnguyen90stnguyen90 requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

@pechtelt1@stnguyen90