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
8263385: IGV: Graph is not opened in the window that has focus. #4078
Conversation
|
/label remove build |
@jyukutyo |
Hello Koichi, thanks for taking care of this issue. I've built and tested this pull request and found that it works in most cases. Here's what did not work:
Despite that I think your change is good. Unfortunately I can only test but not review the change itself as I am not familiar with IGV source code. Thanks, Richard. |
Thank you for confirming, Richard. I appreciate it. |
@reinrich That doesn’t seem doable. I've analyzed the code. Since this pull request can't satisfy the needs, I intend to close this PR. |
@jyukutyo IMHO your change is still a worthwhile. Pity I'm not at all familiar with netbeans and IGV code. Otherwise I'd mark the PR as reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the fix. It works as expected except for a negligible detail of the bug (see PR comment from May 18).
So this looks good to me.
Thanks, Richard.
@jyukutyo This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 805 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@reinrich, @chhagedorn) but any other Committer may sponsor as well.
|
Thank you for the review, Richard. I’m sorry for the delayed response. I'd like to hear the thoughts of other people about this PR and the remaining issue. If they agree with this PR, I will definitely move it forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not even aware of the clone feature - it's nice!
Fix looks good and a manual test worked. I could not reproduce the wrong behavior as mentioned by @reinrich. Nevertheless, even when this is not fixed/not fixable, I think it's still worth to move forward with this PR given that it's a simple fix.
Thank you for reviewing the whole of this PR, Christian. Your comment indeed encouraged me. |
/integrate |
Could someone possibly sponsor this pull request? Thanks, |
/sponsor |
Going to push as commit edff556.
Your commit was automatically rebased without conflicts. |
Thank you for sponsoring it, Yi! |
This pull request enables IGV opens a graph in the window that is focused.
At the moment IGV opens a graph in the window that has the graph and is found first. So in this pull request I used preferentially the active EditorTopComponent.
I tested the following scenarios manually:
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4078/head:pull/4078
$ git checkout pull/4078
Update a local copy of the PR:
$ git checkout pull/4078
$ git pull https://git.openjdk.java.net/jdk pull/4078/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 4078
View PR using the GUI difftool:
$ git pr show -t 4078
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4078.diff