Skip to content

[harmony] Bug 2059621: Add simpler method to check if an extension is present - #66

Open
CyberShadow wants to merge 4 commits into
bugzilla:mainfrom
CyberShadow:have-extension
Open

[harmony] Bug 2059621: Add simpler method to check if an extension is present#66
CyberShadow wants to merge 4 commits into
bugzilla:mainfrom
CyberShadow:have-extension

Conversation

@CyberShadow

Copy link
Copy Markdown
Member

Resurrection of #35.

Applied the suggestions by @dylanwh .

@justdave justdave changed the title Bug 1446236 - Add simpler method to check if an extension is present [harmony] Bug 2059621: Add simpler method to check if an extension is present Jul 31, 2026
@mrenvoize

Copy link
Copy Markdown
Contributor

Should we update the commit message of that first commit to reference the new bug number we have?

Line exceeded the 80-char limit and had inconsistent brace spacing.
Follow-up from QA review of PR bugzilla#66.
@mrenvoize

Copy link
Copy Markdown
Contributor

QA review of this patchset (functional testing in Docker, live smoke test via browser, full sanity suite, perlcritic/perltidy) found no regressions or security concerns. has_extension() matches the semantics of the old grep-based check, and the recursion guard doesn't misfire against any current extension code (none override enabled() or query the extension list at load time).

One minor style nit: the new line in votes.cgi exceeded the project's 80-char limit and had inconsistent brace spacing per perltidy --profile=.perltidyrc. Non-blocking, but flagging as a follow-up:

-Bugzilla->has_extension('Voting') || ThrowCodeError('extension_disabled', { name => 'Voting' });
+Bugzilla->has_extension('Voting')
+  || ThrowCodeError('extension_disabled', {name => 'Voting'});

@mrenvoize mrenvoize left a comment

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.

All looking good here. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants