Skip to content

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

Merged
merged 12 commits into from
May 9, 2017
Merged

Rearrange to expose driver as a lib #7

merged 12 commits into from
May 9, 2017

Conversation

sperand-io
Copy link
Contributor

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)

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
Copy link
Contributor Author

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?

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
Copy link
Contributor Author

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?

Copy link

@tejasmanohar tejasmanohar May 5, 2017

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

@tejasmanohar
Copy link

tejasmanohar commented May 5, 2017

  1. update README install instructions using new path
  2. add go-every back in case someone is relying on it

Copy link

@tejasmanohar tejasmanohar left a 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

@sperand-io
Copy link
Contributor Author

this may be a record in terms of number of commits / LOC changed ratio. @tejasmanohar mind doing another quick review?

ENTRYPOINT ["source-postgres"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@sperand-io sperand-io dismissed tejasmanohar’s stale review May 9, 2017 00:57

because he approved after!

@sperand-io sperand-io merged commit 5a460e3 into master May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants