Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#11393] - Code Style Fixes for administrator\components\com_categories

? Pending

User tests: Successful: Unsuccessful:

avatar photodude
photodude
1 Aug 2016

Pull Request for Code Style Fixes for administrator\components\com_categories.

Summary of Changes

  • Multi-line function call not indented correctly
  • Line exceeds 150 characters
  • Function return type is not void, but function is returning void here
  • Block comment text must start on a new line
  • Multi-line block comment not used
  • Comment must start with a capital letter
  • Expected "integer" but found "int" for @var tag in member variable comment

Automatically fixed with PHPCS2 fixers and some minor edits

Testing Instructions

Merge by code review (or just test that categories still function normally in front end and back end)

avatar joomla-cms-bot joomla-cms-bot - change - 1 Aug 2016
Category ⇒ Administration Components
avatar photodude photodude - open - 1 Aug 2016
avatar photodude photodude - change - 1 Aug 2016
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Aug 2016
Labels Added: ?
avatar zero-24
zero-24 - comment - 2 Aug 2016

Looks good exepted the both comments above. ?

avatar wilsonge
wilsonge - comment - 2 Aug 2016

Seems good. We need to fix the return of the canDelete thing to false otherwise just me being super pedantic

avatar brianteeman
brianteeman - comment - 2 Aug 2016

It's good to be pedantic

avatar wilsonge wilsonge - change - 2 Aug 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 2 Aug 2016
Labels Removed: ?
avatar wilsonge
wilsonge - comment - 2 Aug 2016

RTC

avatar wilsonge wilsonge - change - 2 Aug 2016
Status Pending ⇒ Ready to Commit
avatar wilsonge wilsonge - change - 2 Aug 2016
Milestone Added:
avatar wilsonge
wilsonge - comment - 2 Aug 2016

Bad bot

avatar joomla-cms-bot joomla-cms-bot - change - 2 Aug 2016
Labels Added: ?
avatar rdeutz
rdeutz - comment - 2 Aug 2016

I have heard pedantic :-)

Now canDelete can result with true, false or nothing (what would be cast to false) but we can change it to:


    protected function canDelete($record)
    {
        if (empty($record->id) || $record->published != -2)
        {
            return false;
        }

        return JFactory::getUser()->authorise('core.delete', $record->extension . '.category.' . (int) $record->id);

    }

would make it more clear what is does

avatar photodude
photodude - comment - 3 Aug 2016

@wilsonge What do you think about @rdeutz proposed correction to the canDelete() function?

I think it's better than the existing function, But this PR is already RTC. should I fix it here or should someone do a separate PR?

avatar rdeutz
rdeutz - comment - 3 Aug 2016

@photodude you can commit here, then the bot will remove the RTC and I or someone else can add RTC then, I think there isn't so much danger that we break Joomla with that change :-)

avatar brianteeman brianteeman - change - 3 Aug 2016
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 3 Aug 2016
Labels Added: ?
avatar brianteeman
brianteeman - comment - 3 Aug 2016

The bot didnt remove the label but I have

avatar brianteeman brianteeman - change - 3 Aug 2016
Status Ready to Commit ⇒ Pending
Labels
avatar brianteeman brianteeman - change - 3 Aug 2016
Labels Removed: ?
avatar rdeutz
rdeutz - comment - 3 Aug 2016

I think we can set it on RTC the change is so simple that we don't need the whole process

avatar brianteeman
brianteeman - comment - 3 Aug 2016

@rdeutz your call. I just did what was asked ;)

avatar rdeutz rdeutz - change - 3 Aug 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 3 Aug 2016
Labels Removed: ?
avatar rdeutz rdeutz - change - 3 Aug 2016
Status Pending ⇒ Ready to Commit
avatar rdeutz
rdeutz - comment - 3 Aug 2016

at lease @wilsonge has to decide if he will merge it :-)

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

avatar joomla-cms-bot joomla-cms-bot - change - 3 Aug 2016
Labels Added: ?
avatar rdeutz rdeutz - change - 13 Aug 2016
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2016-08-13 17:53:01
Closed_By ⇒ rdeutz
avatar rdeutz rdeutz - close - 13 Aug 2016
avatar rdeutz rdeutz - merge - 13 Aug 2016
avatar joomla-cms-bot joomla-cms-bot - close - 13 Aug 2016
avatar joomla-cms-bot joomla-cms-bot - change - 13 Aug 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment