Use Windows User Directory for bin path #327
Conversation
|
Repo is not in active development at the moment 😢 #324 |
MatrixCrawler
force-pushed
the
windows-home-dir
branch
7 times, most recently
from
7c21e4b
to
146e77f
Compare
|
Will have to rebase first |
MatrixCrawler
force-pushed
the
windows-home-dir
branch
2 times, most recently
from
2627b96
to
7390720
Compare
|
Done with the rebasing |
lib/defaults.go
Outdated
There was a problem hiding this comment.
Would it make sense to stick with utils.go at once?
https://github.com/warrensbox/terraform-switcher/pull/334/files#diff-bb04fece8dd9230d7af1277a31b0b3c993917058b4343dc7f48f8bcbfe3c8463
There was a problem hiding this comment.
I'm still concerned why would not use utils.go to sync with the other PR https://github.com/warrensbox/terraform-switcher/pull/334/files#diff-bb04fece8dd9230d7af1277a31b0b3c993917058b4343dc7f48f8bcbfe3c8463
Still merge conflict is there |
…ws, as the symlink does not seem to work
…n main and lib. - added detection of home directory in case we are using windows. - updated tests accordingly.
MatrixCrawler
force-pushed
the
windows-home-dir
branch
from
7390720
to
a6a9ae0
Compare
MatrixCrawler
force-pushed
the
windows-home-dir
branch
from
1c576bd
to
fd8dbdc
Compare
|
@yermulnik can you review again? |
lib/symlink.go
Outdated
| log.Fatalf(` | ||
| w, err := os.Create(dir + ".exe") | ||
| if err != nil { | ||
| log.Fatalf(`Could not create target binary: %s.`, dir+".exe") |
There was a problem hiding this comment.
It's the same as #327 (comment) — are backticks intentional?
There was a problem hiding this comment.
Let's use double quotes for consistency?
| } else { | ||
| err := os.Symlink(cwd, dir) | ||
| if err != nil { | ||
| log.Fatalf(` |
There was a problem hiding this comment.
Can backticks be converted to double quotes for consistency (given there are double quotes inside the message — can they be escaped or single quotes used)?
It looks good to me apart 1) ps: Thanks for pinging me as I was waiting for @MatrixCrawler to re-request review once he's done with updates. Apologies for delay. |
@MatrixCrawler @warrensbox Please let me know whether this is meant to be worked on in a new PR and I'll be happy to add my approval to get this merged and released. |
Co-authored-by: George L. Yermulnik <yz@yz.kiev.ua>
Co-authored-by: George L. Yermulnik <yz@yz.kiev.ua>
Co-authored-by: George L. Yermulnik <yz@yz.kiev.ua>
I would see to this in a later PR as i introduce the defaults and the utils in two new PR's. There should be a consolidation, refactoring at a later point. |
I don't see why the consolidation cannot be done in this PR given both are authored by you and this could reduce a number of extra PRs just to merge two files with helper functions 🤷🏻 |
Fixes #199