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

docs: Fix typos and add missing preferences in libraries/email.rst #8851

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

obozdag
Copy link
Contributor

@obozdag obozdag commented May 4, 2024

Description
Fix typos and add missing preferences in libraries/email.rst

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Fix typos and add missing preferences in libraries/email.rst
@kenjis kenjis added the documentation Pull requests for documentation only label May 5, 2024
@kenjis kenjis added the needs rework Changes requested by reviewer that are still pending label Jun 2, 2024
@obozdag
Copy link
Contributor Author

obozdag commented Jun 10, 2024

@kenjis What kind of rework does it need? Should I remove $recipients or $ marks or both?

@kenjis
Copy link
Member

kenjis commented Jun 10, 2024

@obozdag Yes.

Remove recipients from preferences list in libraries/email.rst
@paulbalandan paulbalandan requested a review from kenjis July 23, 2024 16:53
Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

Please remove all $ marks in Preference.
Because these items may be keys in an array.

@kenjis kenjis removed the needs rework Changes requested by reviewer that are still pending label Jul 24, 2024
Remove $ marks in Preference in libraries/email.rst
@obozdag
Copy link
Contributor Author

obozdag commented Jul 24, 2024

@kenjis I have removed all $ marks in Preference.
In https://codeigniter4.github.io/CodeIgniter4/libraries/cookies.html#cookie-personalization there are $ marks and they can be keys in the defaults array. So, shall I also remove the $ marks there?

@kenjis
Copy link
Member

kenjis commented Jul 24, 2024

@obozdag Thank you!

In https://codeigniter4.github.io/CodeIgniter4/libraries/cookies.html#cookie-personalization there are $ marks and they can be keys in the defaults array. So, shall I also remove the $ marks there?

I'm not sure. Because the description for the table is:

However, you may wish to define your own settings by changing the following settings in the Config\Cookie class in app/Config/Cookie.php file.

So it clearly indicates that the table explains properties in Config\Cookie.
But yes, the items are also array keys for Cookie::setDefaults().

@paulbalandan
Copy link
Member

I think I mean here when I made the table is it is referring to the properties of Config\Cookie so the $ should be preserved. The array keys to Cookie::$defaults are private implementation details.

@kenjis kenjis merged commit 8d18ea7 into codeigniter4:develop Jul 25, 2024
4 checks passed
@kenjis
Copy link
Member

kenjis commented Jul 25, 2024

@obozdag Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests for documentation only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants