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

fix: deprecate unity-launch flag #209471

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

rzhao271
Copy link
Contributor

@rzhao271 rzhao271 commented Apr 3, 2024

Fixes #202545

This PR removes mentions of the unity-launch flag.
I have tested the deb and rpm with a custom dev build and found no issues when opening from the launcher with left click or ctrl+left click.

  • Test opening from the dock with ctrl+left click versus opening a terminal from the dock with ctrl+left click

    • Ubuntu deb—both terminal and Insiders open a new instance
    • Ubuntu snap—both terminal and Insiders open a new instance
    • Fedora rpm—both terminal and Insiders open a new instance
  • Test opening from the launcher with ctrl+left click versus opening a terminal from the launcher with ctrl+left click

    • Ubuntu deb—both terminal and Insiders open a new instance
    • Ubuntu snap—both terminal and Insiders open a new instance
    • Fedora rpm—both terminal and Insiders open a new instance in the background
  • Test opening from the dock with shift+left click versus opening a terminal from the dock with shift+left click

    • Ubuntu deb—both terminal and Insiders open a new instance
    • Ubuntu snap—both terminal and Insiders open a new instance
    • Fedora rpm—both terminal and Insiders go to existing instance
  • Test opening from the launcher with shift+left click versus opening a terminal from the launcher with shift+left click

    • Ubuntu deb—both terminal and Insiders go to existing instance
    • Ubuntu snap—both terminal and Insiders go to existing instance
    • Fedora rpm—both terminal and Insiders go to existing instance

@rzhao271 rzhao271 added this to the April 2024 milestone Apr 3, 2024
@rzhao271 rzhao271 requested review from bpasero and deepak1556 April 3, 2024 18:14
@rzhao271 rzhao271 self-assigned this Apr 3, 2024
@rzhao271 rzhao271 force-pushed the rzhao271/remove-unity-launch branch from 1751519 to cdd47fc Compare April 3, 2024 22:27
@rzhao271 rzhao271 force-pushed the rzhao271/remove-unity-launch branch from cdd47fc to 69dee6d Compare April 4, 2024 00:01
@rzhao271
Copy link
Contributor Author

rzhao271 commented Apr 4, 2024

I have confirmed that Insiders seems to behave the same as the terminal in all instances.

@deepak1556 deepak1556 requested review from Tyriar and removed request for deepak1556 April 4, 2024 17:51
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

If --unity-launch happens to be invoked do we refuse to launch now? This could happen if the user forks the code.desktop to customize it.

@rzhao271
Copy link
Contributor Author

rzhao271 commented Apr 4, 2024

@Tyriar I just confirmed that Code-Insiders still launches and warns that the option is not in the list of known options, but is still passed to Electron/Chromium.

@rzhao271 rzhao271 merged commit 0da22a2 into main Apr 4, 2024
19 checks passed
@rzhao271 rzhao271 deleted the rzhao271/remove-unity-launch branch April 4, 2024 22:18
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate the --unity-launch flag
2 participants