Feature/add windows support #87

dmurtic · 2021-07-26T18:23:20Z

For Windows:

Save collie config in C:\Users\<<username>>\AppData\Roaming\.config\collie-cli\
Disable table border because it creates weird ouput
Create Windows artifact in build job
upload windows artifact to release artifacts
add install script for Windows

Close #2

github-actions · 2021-07-26T18:23:39Z

Error in deno fmt. Please check the Action logs.

Jelledb · 2021-07-27T12:27:37Z

Hey Dennis, thanks a lot for your contribution! This also marks our first official external contribution ;-)

I can earliest have a look at Thursday when I'm home and I have access to my personal Windows machine.

tfelix

only the let vs var issue.

src/presentation/mesh-table-factory.ts

src/config/config.model.ts

Jelledb · 2021-07-29T09:34:36Z

install.sh

     ;;
   CYGWIN*|MINGW32*|MSYS*|MINGW*)
-     echo 'Collie currently does not support Windows. Please have a look at https://github.com/meshcloud/collie-cli/issues/2 to follow progress.'
+     echo 'Please use install.ps1 script. See readme for more information.'


Suggested change

echo 'Please use install.ps1 script. See readme for more information.'

echo 'Please execute the install.ps1 script as mentioned in our README. Go to https://github.com/meshcloud/collie-cli#-install-and-usage'

Jelledb · 2021-07-29T09:39:05Z

install.ps1

+New-Item -ItemType directory -Path "${HOME}\collie-cli"
+Invoke-WebRequest -Uri $url -OutFile "${HOME}\collie-cli\collie.exe"
+
+Write-Host "Please add ${HOME}\collie-cli\collie.exe to your path environemnt variable."


Suggested change

Write-Host "Please add ${HOME}\collie-cli\collie.exe to your path environemnt variable."

Write-Host "Please add ${HOME}\collie-cli\collie.exe to your path environment variable."

For me to get collie as a command to work I had to:

Remove the .exe part from the file name, as it forces me to run collie.exe tenant list otherwise.

Remove collie.exe from the path. Instead, I added C:/Users/Jelle/collie-cli/ to the path. Other approaches did not work.

Jelledb · 2021-07-29T09:50:23Z

I'll be testing the working of the application now.

So far:

The initial output from INFO, which is blue, looks very awkward in powershell. I can barely read it.
Auto detecting az does not work upon setup. (might be a problem with the others too, haven't tested)
It also gives a strange output when you have nothing installed, it says: I see you already have the cloud CLIs installed: .. I think we should just skip this step if no CLIs are installed.
I cannot get beyond setup. It crashes at the end with The system cannot find the path specified. (os error 3).

Jelledb · 2021-07-29T16:41:55Z

Fixed all mentioned above except for blue logging.

tfelix

lgtm

tfelix · 2021-07-30T08:58:28Z

src/commands/init-commands.ts

    .name(CLICommand)
+    .help({
+      // The darkblue of Cliffy doesn't look great on the blue of PowerShell.
+      colors: Deno.build.os !== "windows",


d: Maybe we should introduce a global variable like "isWindows: bool" or something?

tfelix · 2021-07-30T09:00:46Z

src/presentation/mesh-table-factory.ts


  buildMeshTable(): MeshTable {
-    if (this.isatty) {
+    if (this.isatty && os.default.platform() !== "windows") {


d: we mix Deno.build.os !== "windows" and os.default.platform() !== "windows" maybe really consider this central variable at some meaningful spot, maybe in one of the init.ts files?

makes total sense, will do

tfelix · 2021-07-30T09:01:51Z

src/init.ts

      }
    });
-    const answer = await askYesNo(
+    const answer: boolean = await Confirm.prompt(


interesting. I guess this works on Linux as well? But looks good to me.

Yes, we used it already. That's why I decide to use it.
This confirm prompt is also used when asking to install some azure extension (I think you wrote that)

github-actions · 2021-07-30T09:09:11Z

Error in deno fmt. Please check the Action logs.

github-actions · 2021-07-30T09:09:12Z

Error in deno lint. Please check the Action logs.

github-actions · 2021-07-30T09:19:09Z

Error in deno fmt. Please check the Action logs.

* feat: add windows support for collie * feat: add windows binary to releases * fix: differentiation in posix and win32 path * fix: Replaces var with let * Fixed issue found while testing Windows * Added global `isWindows` var for consistency Co-authored-by: Thomas Felix <tfelix@meshcloud.io> Co-authored-by: Jelle den Burger <jdburger@meshcloud.io>

* feat: Add tutorial links directly in README. (#52) * feat: Add tutorial links directly in README. * And removed GitOps section, which is not supported yet. * Update README.md * Update README.md Added a different caption * feat: Avoid cli setup checks when not required (#68) * feat: Avoid cli setup checks when not required In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required. * fix: setup logger before running commands and await it not awaiting the logger configuration can cause loosing logs * feat: run cli checks concurrently see discussion in #68 * feat: add debug logs to ShellRunner for every command * style: deno fmt Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io> * Feature/issue 56 (#71) * feat: detect installed cli versions match expectations if not, give installation hints via collie readme. fixes #56 * fix: handle GCP projects with empty IAM policies (#80) fixes #67 * fix: use azure subscription id instead of name to prevent word-splitting (#79) fixes #76 * Feature/cache statistics (#78) * refactor: generate entries summary generically for all platforms * feat: add query statistics to tty table output This lets users know whether results are served from cache or if they're queried from the cloud. At the moment only "tenant list" command supports this. Fixes #72 * feat: add query statistics to all tenant commands Fixes #72 * refactor: extract QueryStatistics class into its own module * test: add a tests for query statistics * feat: slightly tweak the query stats message. * feat: correctly print cache and cloud query statistics for hybrid query hybrid queries can occur when e.g. running - collie tenant list -> partially fills the tenant cache - collie tenant iam -> uses tenants from cache, adds IAM info * refactor: Uses OOP patterns Simplifies the statistics decorator to be less intrusive. Co-authored-by: Thomas Felix <tfelix@meshcloud.io> * Update issue templates * Update issue templates * fix: Spinner animation overwrites error message (#85) Improves the spinner animation termination to avoid part of the error message writing into the spinner animation text. * feat: Tags are shown in table cost view (#90) * fix: Fix costs / IAM missing in cached tenants. (#91) * Feature/add windows support (#87) * feat: add windows support for collie * feat: add windows binary to releases * fix: differentiation in posix and win32 path * fix: Replaces var with let * Fixed issue found while testing Windows * Added global `isWindows` var for consistency Co-authored-by: Thomas Felix <tfelix@meshcloud.io> Co-authored-by: Jelle den Burger <jdburger@meshcloud.io> * Bump version to 0.5.0 Co-authored-by: Thomas Felix <1186998+tfelix@users.noreply.github.com> Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io> Co-authored-by: Thomas Felix <tfelix@meshcloud.io> Co-authored-by: Dennis Murtic <73823855+dmurtic@users.noreply.github.com>

dmurtic requested a review from Jelledb July 26, 2021 18:23

github-actions bot added the feature label Jul 26, 2021

dmurtic force-pushed the feature/add-windows-support branch 3 times, most recently from f85d3c5 to 62a040c Compare July 26, 2021 18:44

dmurtic requested a review from tfelix July 26, 2021 18:45

tfelix approved these changes Jul 29, 2021

View reviewed changes

src/presentation/mesh-table-factory.ts Outdated Show resolved Hide resolved

src/config/config.model.ts Outdated Show resolved Hide resolved

Jelledb requested changes Jul 29, 2021

View reviewed changes

Dennis Murtic and others added 4 commits July 29, 2021 18:47

feat: add windows support for collie

8b41594

feat: add windows binary to releases

5ae9aae

fix: differentiation in posix and win32 path

8de02b9

fix: Replaces var with let

c2c7590

Jelledb force-pushed the feature/add-windows-support branch 2 times, most recently from 7ce2d5f to ab1bcc3 Compare July 30, 2021 08:17

Fixed issue found while testing Windows

83cf941

Jelledb force-pushed the feature/add-windows-support branch from ab1bcc3 to 83cf941 Compare July 30, 2021 08:26

tfelix approved these changes Jul 30, 2021

View reviewed changes

Jelledb force-pushed the feature/add-windows-support branch 2 times, most recently from 639440d to 7ab1e67 Compare July 30, 2021 09:18

Added global isWindows var for consistency

464ee21

Jelledb force-pushed the feature/add-windows-support branch from 7ab1e67 to 464ee21 Compare July 30, 2021 09:20

Jelledb merged commit 7f060b1 into develop Jul 30, 2021

Jelledb deleted the feature/add-windows-support branch July 30, 2021 09:21

	echo 'Please use install.ps1 script. See readme for more information.'
	echo 'Please execute the install.ps1 script as mentioned in our README. Go to https://github.com/meshcloud/collie-cli#-install-and-usage'

	Write-Host "Please add ${HOME}\collie-cli\collie.exe to your path environemnt variable."
	Write-Host "Please add ${HOME}\collie-cli\collie.exe to your path environment variable."

Feature/add windows support #87

Feature/add windows support #87

Uh oh!

Conversation

dmurtic commented Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 26, 2021

Uh oh!

Jelledb commented Jul 27, 2021

Uh oh!

tfelix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jelledb Jul 29, 2021

Choose a reason for hiding this comment

Uh oh!

Jelledb Jul 29, 2021

Choose a reason for hiding this comment

Uh oh!

Jelledb Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jelledb commented Jul 29, 2021

Uh oh!

Jelledb commented Jul 29, 2021

Uh oh!

tfelix left a comment

Choose a reason for hiding this comment

Uh oh!

tfelix Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

tfelix Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

Jelledb Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

tfelix Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

Jelledb Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 30, 2021

Uh oh!

github-actions bot commented Jul 30, 2021

Uh oh!

github-actions bot commented Jul 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dmurtic commented Jul 26, 2021 •

edited

Loading

Jelledb Jul 29, 2021 •

edited

Loading