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

extensions/BugModal: Enable bugzilla-readable-status unconditionally #74

Merged
merged 5 commits into from
Feb 1, 2024

Conversation

CyberShadow
Copy link
Member

I see no downside for enabling it always, and let it generate readable descriptions for the "Tracking" section. They appear useful even without the TrackingFlags extension.

This also removes some hard-coded BMO/Firefox-specific logic.

Copy link
Member

@dylanwh dylanwh left a comment

Choose a reason for hiding this comment

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

Please remove the if (1) { ... } wrapping, that's not needed. I agree with this change other than this small nit.

I see no downside for enabling it always, and let it generate readable
descriptions for the "Tracking" section. They appear useful even
without the TrackingFlags extension.
@CyberShadow
Copy link
Member Author

Done :)

Base automatically changed from master to main February 13, 2021 18:03
@justdave justdave requested a review from dylanwh March 21, 2021 05:19
@justdave justdave dismissed dylanwh’s stale review March 21, 2021 05:20

requested chages were made

@justdave
Copy link
Member

Do we have a Bugzilla bug that goes with this PR?

@justdave
Copy link
Member

justdave commented Sep 4, 2023

Still don't see a bug filed to go with this.
If we hope to keep BMO following upstream in the future, shouldn't we move this code into the BMO extension and add a hook in this spot that it can hook to insert it back?

@CyberShadow
Copy link
Member Author

Sorry, this is part of the BMO cleanup effort, so same bug as most other PRs from me.

@CyberShadow
Copy link
Member Author

CyberShadow commented Sep 4, 2023

If we hope to keep BMO following upstream in the future,

My understanding was that this was never an option, as the BMO team is uninterested in collaborating with any upstream. Has this changed?

@justdave
Copy link
Member

justdave commented Sep 4, 2023

My understanding was that this was never an option, as the BMO team is uninterested in collaborating with any upstream.

Yeah, I guess that's pretty much the way it is. They can cherry-pick if they want our stuff. And we'll do the same to theirs in the future.

@justdave
Copy link
Member

justdave commented Feb 1, 2024

Sorry, this is part of the BMO cleanup effort, so same bug as most other PRs from me.

OK, so bug 1446236

@justdave justdave merged commit 37f6a83 into bugzilla:main Feb 1, 2024
4 of 6 checks passed
@justdave
Copy link
Member

FYI, this commit broke filing new bugs.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1892558

@justdave
Copy link
Member

correction, it broke show_bug, not filing new ones.

@justdave
Copy link
Member

This appears to have been missing from the patch: (I made this up based on the usage, but it seems to work)

diff --git a/Bugzilla.pm b/Bugzilla.pm
index e58d84f32..06aa3fb0a 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -135,6 +135,12 @@ sub extensions {
   return $extensions;
 }

+sub has_extension {
+  my (undef, $extension) = @_;
+  return 1 if (grep { $_ eq $extension } Bugzilla->extensions);
+  return 0;
+}
+
 sub cgi {
   return request_cache->{cgi} ||= Bugzilla::CGI->new;
 }

However, that leads to:

Odd number of elements in anonymous hash at /app/extensions/BugModal/Extension.pm line 209.

I don't think this was ready to land.

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