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

[SHACK-353] Enable CWA on Windows #241

Merged
merged 3 commits into from
Sep 13, 2018

Conversation

marcparadise
Copy link
Member

@marcparadise marcparadise commented Sep 8, 2018

Description

This adds a "Chef Workstation App" feature to the Windows
Installer. It provides a 'run on login' component that will
add CWA to common startup

If Start menu items are selected, it will include this
shortcut there as well.

This branch also renames the shell shortcut "Chef Workstation" to "Chef
Workstation Command Prompt". This is more in line with recent
discussions around naming, and avoids confusion between a shortcut
called "Chef Workstation" and one named "Chef Workstation App".

This also changes the CWA tray app package location to $install_path/components/chef-workstation-app instead of $install_path/installers - it's more accurate.

Check List

@marcparadise marcparadise requested a review from a team September 8, 2018 04:53
@marcparadise marcparadise mentioned this pull request Sep 10, 2018
6 tasks
@marcparadise marcparadise force-pushed the SHACK-353/windows-ws-app-setup branch 4 times, most recently from e45b2df to a0f874e Compare September 11, 2018 20:18
@marcparadise marcparadise changed the title Add Windows installer options for Chef Workstation App [SHACK-353] Add Windows installer options for Chef Workstation App Sep 12, 2018
@marcparadise marcparadise changed the title [SHACK-353] Add Windows installer options for Chef Workstation App [SHACK-353] Enable CWA on Windows Sep 12, 2018
@marcparadise
Copy link
Member Author

This should be held out of merge until after #7

@marcparadise marcparadise force-pushed the SHACK-353/windows-ws-app-setup branch 2 times, most recently from ce404ea to d46cc2d Compare September 12, 2018 18:28
@marcparadise
Copy link
Member Author

This is pending a commit clean-up - some of the localization changes got pushed to the wrong branch.

Copy link
Contributor

@jonsmorrow jonsmorrow left a comment

Choose a reason for hiding this comment

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

Looks ok to me.

Copy link
Contributor

@tyler-ball tyler-ball left a comment

Choose a reason for hiding this comment

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

A few questions just to make sure I understand this. But overall LGTM! And the most important thing is whether it works

<Environment Id="Environment"
Name="PATH" Action="set" Part="last" System="yes" Value="[PROJECTLOCATIONBIN]" />
</Component>
<Component Id="StartChefWSPowershellShortcut" Guid="{69EF5736-ED67-45B3-887A-FBFC0AB6DA13}" >
Copy link
Contributor

Choose a reason for hiding this comment

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

Nix me if this is pedantic. But could we refer to this as ChefWsApp and start-chef-ws-app.ps1 in this component to be most clear?

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't for the app - this is the config for the powershell shortcut. I can rename the component itself though, "start-ws.ps1" isn't really "Start Chef Workstation". chef-ws-powershell.ps1?

I will do this in a separate minor PR to avoid more rebasing of downstream branches.

</Component>
</Directory>
<Directory Id="COMPONENTS" Name="components">
<Directory Id="WSAPPDIR" Name="chef-workstation-app">
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this Directory tag doing? Is it creating components/chef-workstation-app as part of the install? Or just listing it so that Wix knows about it and could (EG) delete it on uninstall?

Copy link
Member Author

Choose a reason for hiding this comment

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

We reference it later as WSAPPDIR in the CWA shortcut setup, to provide the location for the binary.

This changes the installed location of chef-workstation-app to
$install_dir/components/chef-workstation-app as we switch over
to including the unpacked application in the installer package.

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
This splits the Chef Workstation windows installer into
'Chef Workstation' and 'Experimental Features'.

'Chef Workstation' contains the existing items for this package,
while 'Experimental Features' currently provides the option to
install Chef Workstation Tray.  Selecting this will install
autostart, desktop, and Start shortcuts for it.

This is intended to be a short-term approach while we work on
moving these types of feature flags into configuration and the
Workstation App itself.

Additional changes:

* renames the shell shortcut "Chef Workstation" to "Chef
  Workstation Command Prompt". This is more in line with recent
  discussions around naming, and avoids confusion between a shortcut
  called "Chef Workstation" and one named "Chef Workstation App".
* adds clarifying wording to Windows installation components and
  descriptions now that there can be more than one 'Chef Workstation' icon
* shorten key names

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
Also applies consistent formatting to source.wxs.erb

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
@marcparadise marcparadise merged commit feb112c into master Sep 13, 2018
@chef-ci chef-ci deleted the SHACK-353/windows-ws-app-setup branch September 13, 2018 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants