-
Notifications
You must be signed in to change notification settings - Fork 0
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
Provide working CLI for existing Twitter actions #29
Provide working CLI for existing Twitter actions #29
Conversation
For parsing CLI arguments it could've been helpful to use As using Node.js 16, some external options are available: |
1e6c01d
to
3be78a4
Compare
d78fcee
to
e707d23
Compare
@@ -0,0 +1,28 @@ | |||
#!/usr/bin/env node | |||
|
|||
import meow from "meow"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESM imports look nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on #31
4e68a09
to
051322e
Compare
051322e
to
fa2d41f
Compare
76b78c0
to
9138adf
Compare
CLI implementation weighs more than it does, lacks tests, and the design feels not scalable. Though it's likely to be replaced completely when it's clear where the software goes. For now, it's fine to merge as long as it somehow works. |
const page = await context.browserContext.newPage(); | ||
const profileView = new UserProfileViewAdapter(page); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like BrowserContext
would be better injectable here, so the view itself can instantiate the page, etc.
Addresses #20