-
Notifications
You must be signed in to change notification settings - Fork 40
Conversation
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.
looks good, few comments.
Makefile
Outdated
curl -L https://github.com/cloudquery/cloudquery/releases/latest/download/cloudquery_Darwin_x86_64 -o cloudquery | ||
chmod +x ./cloudquery | ||
|
||
install-cq-mac: |
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.
what's the difference between install-cq
, install-cq-mac
(twice). I think we can get the OS and the arch automatically and put it in an env?
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.
If we can't, how about mac-m1? Or how about this:
install-cq:
@echo Go to https://docs.cloudquery.io/install-instructions and do the thing there please
(ie. I'm questioning whether it's needed to have it here)
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.
I left install-cq
target in there because that is what I use. I would love to support automatically detecting arch and os.
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.
I added the instructions under this target. When we can support dynamic detection we can replace it with the automated install
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.
LGTM
Make it easier to develop the
cq-provider-aws
you can run a specific e2e test like this: