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

Mini DOCBlock update #4415

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

sreichel
Copy link
Contributor

No description provided.

@github-actions github-actions bot added Component: Core Relates to Mage_Core Component: Eav Relates to Mage_Eav Mage.php Relates to app/Mage.php labels Dec 13, 2024
@sreichel sreichel force-pushed the minor-fixes-magerun branch from 1aedd15 to 8bbdfff Compare December 14, 2024 03:48
@github-actions github-actions bot added the Component: Directory Relates to Mage_Directory label Dec 14, 2024
@sreichel sreichel force-pushed the minor-fixes-magerun branch from 8bbdfff to c9a1db8 Compare December 14, 2024 03:56
@github-actions github-actions bot added the Component: Catalog Relates to Mage_Catalog label Dec 14, 2024
@sreichel sreichel force-pushed the minor-fixes-magerun branch from c9a1db8 to 8252c97 Compare December 14, 2024 04:00
@github-actions github-actions bot added the Component: Rss Relates to Mage_Rss label Dec 14, 2024
@sreichel sreichel marked this pull request as draft December 15, 2024 05:44
@sreichel sreichel marked this pull request as ready for review December 18, 2024 06:37
@sreichel
Copy link
Contributor Author

Fixes some issues found during work on magerun v3 release.

netz98/n98-magerun#1484

@sreichel
Copy link
Contributor Author

Ignore CS, fixed in #4425

@sreichel sreichel requested review from kiatng and addison74 December 19, 2024 09:25
Comment on lines -48 to +51
* @method bool getIsActive()
* @method $this setIsActive(bool $value)
* @method bool getIsAnchor()
* @method $this setIsAnchor(bool $value)
* @method int getIsActive()
* @method $this setIsActive(int $value)
* @method int getIsAnchor()
* @method $this setIsAnchor(int $value)
Copy link
Member

Choose a reason for hiding this comment

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

Are there going to be negative consequences of putting the return type we wish we had instead of the one we actually have? The return type is actually likely to be string "0|1" because of the horrible decision of someone long ago to make PDO return strings for all data types. But it could also be bool or int because of inconsistencies in the use of the setter. That is:

$category = Mage::getModel('catalog/category')->load(1);
var_dump($category->getIsActive()); // string "0|1"
$category->setIsActive(true);
var_dump($category->getIsActive()); // bool true

Copy link
Contributor Author

@sreichel sreichel Dec 20, 2024

Choose a reason for hiding this comment

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

No. In the past, when i added method annotions i took the values i have found. E.g. bool, but in PDO its either string or int. bool is wrong for DB values .

magerun uses (int) constants for active/inactive, thats looks more correct to me.

Id like to continue #4376 and add getter/setter methods to reflect the correct type ... later.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, just something to be thinking about. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Component: Catalog Relates to Mage_Catalog Component: Core Relates to Mage_Core Component: Directory Relates to Mage_Directory Component: Eav Relates to Mage_Eav Component: Rss Relates to Mage_Rss Mage.php Relates to app/Mage.php
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants