-
-
Notifications
You must be signed in to change notification settings - Fork 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
Fix URLs openings #69
Conversation
Following .net core migration replacing Process.Start() method to open URLs by the OsShellUtil.OpenUrlInDefaultBrowser() one
Hello! Any chance to see this pull request merged and the plugin released? Thanks in advance |
@@ -12,19 +11,9 @@ public FormPluginInformation() | |||
InitializeComplete(); | |||
} | |||
|
|||
public static void ShowSubmitted(IWin32Window owner, string change) |
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.
Can you explain this code deletion? Was it dead code?
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.
No problem! 😉
It seams that this is a bad copy paste from the FormGerritChangeSubmitted.cs file that has exactly the same method with same body and where the location regarding the method name seems to make more sense
Moreover a "Find reference" on this method returns an empty result so I consider It was dead code
Sorry @poupounetjoyeux I wrote a comment but forgot to submit the review. |
I will let @lekhmanrus merge it and do a release if he thinks it's the good moment to do one... |
Yes sure! Thanks for the feedback ! 😉 |
Following .net core migration replacing Process.Start() method to open URLs by the OsShellUtil.OpenUrlInDefaultBrowser() one
Thanks to @pmiossec for the solution
PR Checklist
Please check if your PR fulfills the following requirements:
Bug fix pretty difficult to test (don't want to open a browser) and doesn't really need documentation
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Following the .net core migration we are unable to open browser links (for Gerrit documentation or on change submitted) from the Gerrit plugin
Issue Number: #62
What is the new behavior?
Using the OsShellUtil.OpenUrlInDefaultBrowser() that is .net core compatible to open URLs
Does this PR introduce a breaking change?