Wire login and identify #13
Conversation
jonallured
force-pushed
the
wire-login-and-identify
branch
from
df86f7b
to
eac5de5
Compare
src/commands/login.ts
Outdated
| @@ -23,6 +25,8 @@ export default class Login extends Command { | |||
| password, | |||
| }) | |||
|
|
|||
| const path = `${os.homedir()}/.config/artsy` | |||
| fs.writeFileSync(path, result.access_token) | |||
There was a problem hiding this comment.
Perhaps consider ~/.netrc ? There’s probably libs for that too.
There was a problem hiding this comment.
Re: .netrc, we have a ticket for that: https://github.com/artsy/artsy-cli/projects/1#card-24981275. I think stuffing it into .config is an iteration toward that goal.
There was a problem hiding this comment.
Yeah, this PR was intentionally naive, but then I got spec failures and now I'm sorta changing my mind. I've got some work locally where I used testdouble.js to mock out fs and os, but then I run specs and everything blows up, haha. I should have seen this coming, but when you mock those out, they are mocked for all the code, not just the SUT. I learned this lesson before with pear but then forgot.
https://github.com/jonallured/pear/blob/master/test/pear-data.test.ts
With pear I actually extracted some units that wrap fs and os and are under my control that I could then safely mock. Going to take a step back and work on that so that how we persist does not matter to this test. We could use ~/.netrc or a config file or whatever.
There was a problem hiding this comment.
With something like https://www.npmjs.com/package/netrc you only need to mock that API, FS tests are already done by the lib.
jonallured
force-pushed
the
wire-login-and-identify
branch
from
a55e2c0
to
b55cfbe
Compare
This is about as DIY as it gets, but is really all we need in order to mock out the config layer. If/when we add a proper mocking library it would probably be good to migrate this over too.
Ya know what I mean?
jonallured
force-pushed
the
wire-login-and-identify
branch
from
b55cfbe
to
db1e105
Compare
|
Ok this ended up needing more hand-holding than I expected, but I was able to get a bunch of stuff done here:
Going to merge this for now and then would be happy to keep iterating on this stuff! |
|
🚀 PR was released in v0.0.5 🚀 |
1 similar comment
|
🚀 PR was released in v0.0.5 🚀 |
|
🚀 PR was released in v0.0.5 🚀 |
2 similar comments
|
🚀 PR was released in v0.0.5 🚀 |
|
🚀 PR was released in v0.0.5 🚀 |
This PR wires together the login and identify commands. All I'm doing here is writing the token to disk and then reading it back in.