Skip to content
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

several fixes #57

Merged
merged 6 commits into from
Nov 19, 2018
Merged

several fixes #57

merged 6 commits into from
Nov 19, 2018

Conversation

timotheecour
Copy link
Contributor

@timotheecour timotheecour commented Nov 16, 2018

/cc @yglukhov @SSPkrolik

@timotheecour timotheecour changed the title test/document Bson(string) to string several fixes Nov 16, 2018
Copy link
Contributor

@yglukhov yglukhov left a comment

Choose a reason for hiding this comment

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

You forgot to change .travis.yml and I wonder how tests succeeded :/

@timotheecour
Copy link
Contributor Author

hmm, indeed, nim c -r nimongo/bsontest is mentioned in .travis.yml and doesn't fail... sounds like a pre-existing bug

@timotheecour
Copy link
Contributor Author

@SSPkrolik @yglukhov I tested locally (based on output from https://travis-ci.com/timotheecour/nimongo/jobs/159187423) , it turns out travis.yml is buggy, can we please merge this and figure out travis after?

mkdir /tmp/d02 && cd /tmp/d02
git clone --depth=50 --branch=master https://github.com/timotheecour/nimongo.git timotheecour/nimongo
docker pull yglukhov/nim-base

# passes
docker run -v "$(pwd):/project" -w /project yglukhov/nim-base run "nimble install -y && nim c -r nimongo/bson && nim c -r nimongo/bsontest"

# also passes, with random string
docker run -v "$(pwd):/project" -w /project yglukhov/nim-base run "nimble install -y && nim c -r nimongo/bson && nim c -r adfasfjalfjaslfjasdlfjalsjfalsjfasljfalsdfj"

@timotheecour
Copy link
Contributor Author

timotheecour commented Nov 16, 2018

@SSPkrolik @yglukhov
PTAL, I fixed travis, there was a pre-existing bug (run vs bash); also I adapted travis to run nimble test_ci so we can test both nimble test (locally, if there's a mongod server) vs nimble test_ci

in a future PR, we should make travis run nimble test and add dependency on mongod in travis => #62

@timotheecour
Copy link
Contributor Author

/cc @yglukhov I added some commits ; lemme know if u want these in another PR

@timotheecour
Copy link
Contributor Author

ping

@SSPkrolik SSPkrolik merged commit 777cb55 into SSPkrolik:master Nov 19, 2018
@SSPkrolik
Copy link
Owner

@timotheecour thanks a lot for the contribution!

@timotheecour timotheecour deleted the pr_fixes branch November 19, 2018 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants