Skip to content

Task/add acp options#5

Closed
phpbbireland wants to merge 18 commits intophpbb-extensions:masterfrom
phpbbireland:task/add-acp-options
Closed

Task/add acp options#5
phpbbireland wants to merge 18 commits intophpbb-extensions:masterfrom
phpbbireland:task/add-acp-options

Conversation

@phpbbireland
Copy link
Copy Markdown

Should be what's required but added permissions too... tested and working

Comment thread event/listener.php Outdated
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.

There is no need for a controller_helper.

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.

bump

@iMattPro
Copy link
Copy Markdown
Contributor

iMattPro commented Jul 8, 2014

Just a general note. Please review https://gist.github.com/VSEphpbb/973c06e22abd7c2427c3 as it is already pretty much a working version of the ACP event functions.

Also, since you are starting to use language files you should also add them, meaning this PR should also have an info_acp_language file with any of the language variables introduced in this PR.

The same goes for the services.yml, there needs to be the event listener service with the configs defined in the constructor of the event listener.

Manual update using git gitsfile but including permissions
Are permissions not needed?
Removing unwanted vars
Copyright date is 2014?
Comment thread event/listener.php Outdated
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.

controller_helper should be removed

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.

you need the $template var, it's being used now in insert_googleanalytics_id()

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.

3 participants