-
Notifications
You must be signed in to change notification settings - Fork 7
Rearrange to expose driver as a lib #7
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
Conversation
Dockerfile
Outdated
# Additionally add a cron-like runner to run on an | ||
# interval. | ||
RUN go get -u github.com/segmentio/go-every | ||
ADD . /go/src/github.com/segment-sources/postgres |
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.
no idea where i should be putting this. @tejasmanohar is there like a convention i should be using 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.
location is fine. didn't know ADD
will mkdir
for you... just docker build . -> docker run and it worked though!
Dockerfile
Outdated
|
||
# Additionally add a cron-like runner to run on an | ||
# interval. | ||
RUN go get -u github.com/segmentio/go-every |
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.
dont think this was actually being used?
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.
it's there in mongo source too. i'd keep the dep as is for this PR in case people are doing docker run --entrypoint=every postgres --...
or something
|
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.
README fix, otherwise LGTM
this may be a record in terms of number of commits / LOC changed ratio. @tejasmanohar mind doing another quick review? |
ENTRYPOINT ["source-postgres"] |
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.
nice
it's likely folks will want to consume the driver separately so that they can write their own runners. in fact, we do :D (for bookkeeper etl)