Skip to content

Bug 103866: show bugs blocked by unresolved dependencies #8

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

justdave
Copy link
Member

@justdave justdave commented Jan 25, 2018

https://bugzilla.mozilla.org/show_bug.cgi?id=103866

Not tested, but the code looks clean and looks like it should do what it says on the tin.
Roping in Kyohei to look because it's search UI (well, buglist UI really - it's just an additional column you can pick to display).

@justdave justdave requested review from dylanwh and kyoshino January 25, 2018 06:53
dylanwh
dylanwh previously approved these changes Mar 5, 2018
@dylanwh dylanwh changed the base branch from master to unstable March 16, 2018 01:41
@mohawk2
Copy link

mohawk2 commented Apr 8, 2018

I see "issues":

  • Is there anything that can be done to tidy up this storm of commits and merge-commits?
  • The merge conflicts need fixing
  • The tests need to be passing
  • Can the .gitignore change for .DS_Store get put onto master and all other main branches to eliminate this as a thing showing in PRs?

@dylanwh
Copy link
Member

dylanwh commented Apr 8, 2018

@justdave can you rebase this against the unstable branch, and then force push it?

@justdave
Copy link
Member Author

@justdave can you rebase this against the unstable branch, and then force push it?

There's a lot of conflicts in code that I have no clue how it works and don't know what the intended change was to know what to merge.

@justdave
Copy link
Member Author

Looks like something got merged here that wasn't meant to be, the conflicts all have nothing to do with the patch. What branch should be the target for this merge? It's been a couple years since this was posted. Is unstable still appropriate or should it go on main?

@justdave justdave force-pushed the bug-103866-query-on-blocked-somewhere branch from cf25e58 to 803ab2e Compare August 14, 2021 07:54
@justdave
Copy link
Member Author

OK, I re-created my local branch from unstable, then cherry-picked this patch off the original upstream branch, then force-pushed it back.

* Is there anything that can be done to tidy up this storm of commits and merge-commits?

Pretty much what I just did.

@justdave justdave requested review from dylanwh and removed request for kyoshino August 14, 2021 08:02
@justdave justdave dismissed dylanwh’s stale review August 14, 2021 08:03

Want this re-approved to ensure it's going on the correct target branch

Comment on lines 643 to 636
blocked_somewhere =>
"(CASE WHEN bugs.bug_id IN"
. " (SELECT blocked FROM dependencies JOIN bugs ON (dependencies.dependson = bugs.bug_id)"
. " WHERE bug_status IN"
. " (SELECT value FROM bug_status WHERE is_open = 1) GROUP BY blocked)"
. " THEN 1"
. " ELSE 0"
. " END)",
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to worry about the potential performance impact here? (DoS potential etc.)

@CyberShadow
Copy link
Member

Want this re-approved to ensure it's going on the correct target branch

I don't think we use the unstable branch any more, do we? I think the last commit there predates the most recent BMO sync.

@justdave
Copy link
Member Author

justdave commented Jan 7, 2022

@dylanwh ping

@justdave
Copy link
Member Author

Confirmed with Dylan on Discord that unstable is dead and this should be rebased against main.

@justdave justdave changed the base branch from unstable to main February 18, 2022 20:06
@justdave
Copy link
Member Author

Which is going to require repeating the steps from #8 (comment) again...

@justdave justdave force-pushed the bug-103866-query-on-blocked-somewhere branch from 803ab2e to 97782ef Compare February 18, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants