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

[4.0] Correct @param docblock parameter types #29882

Merged
merged 6 commits into from
Jul 11, 2020
Merged

[4.0] Correct @param docblock parameter types #29882

merged 6 commits into from
Jul 11, 2020

Conversation

PhilETaylor
Copy link
Contributor

Summary of Changes

Method signature changes in docblock only, no code changes

Actual result BEFORE applying this Pull Request

The docblock documented that $id could only be an integer

This is factually incorrect as evidenced by the second param $byName which is a bool that says if the $id is the name (i.e. a string "site/administrator/api/cli etc") then to handle it differently than an integer client id.

There are many places in code where $id is provided as a string of the name and $byName is set to true, further proving the documentation to be incorrect.

Documentation Changes Required

None.

@ChristineWk
Copy link

I have tested this item ✅ successfully on c591762


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.

@ChristineWk
Copy link

I have tested this item ✅ successfully on 53d213a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.

@Quy
Copy link
Contributor

Quy commented Jul 2, 2020

From Joomla! Coding Standards

Whereas normal code indenting uses real tabs, all whitespace in a Docblock uses real spaces. This provides better readability in source code browsers. The minimum whitespace between any text elements, such as tags, variable types, variable names and tag descriptions, is two real spaces.

@ChristineWk
Copy link

ChristineWk commented Jul 2, 2020

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.
Had the wrong button.

@ChristineWk
Copy link

I have tested this item ✅ successfully on f04e476


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jul 7, 2020

I have tested this item ✅ successfully on f04e476


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.

@Quy
Copy link
Contributor

Quy commented Jul 7, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29882.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 7, 2020
@richard67 richard67 merged commit f28b3b9 into joomla:4.0-dev Jul 11, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 11, 2020
@richard67
Copy link
Member

Thanks!

@richard67 richard67 added this to the Joomla 4.0 milestone Jul 11, 2020
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
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.

None yet

6 participants