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

Made changes to allow the API to work with cual-id, UUID format #34

Merged
merged 5 commits into from Aug 23, 2016

Conversation

kmckinnis
Copy link
Contributor

As said above

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.407% when pulling 984c1cb on kmckinnis:master into 1bb93ae on johnchase:master.

def test_create_ids_API(self):
result = list(create_ids(100, 7))
self.assertTrue(len(str(result[0][0])) < 10)
self.assertTrue(len(str(result[0][1])) > 10)
Copy link
Owner

Choose a reason for hiding this comment

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

a very minor comment, but will you change these to use the == operator rather than the greater than less than for the cualid and then check the type for the uuid?

id_length = 7
result = list(create_ids(100, id_length))
self.assertTrue(len(result[0][0]) == id_length)
self.assertTrue(type(result[0][1]) == uuid.UUID)

@johnchase
Copy link
Owner

Thanks @kmckinnis One very minor comment, otherwise looks good!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.407% when pulling dc3901a on kmckinnis:master into 1bb93ae on johnchase:master.

@johnchase
Copy link
Owner

@kmckinnis Will you make the changes as per the code block I included in my last comment?

@kmckinnis
Copy link
Contributor Author

Sorry I didn't see that part of the email. Didn't hit read more.

On Mon, Aug 22, 2016 at 4:29 PM, John Chase notifications@github.com
wrote:

@kmckinnis https://github.com/kmckinnis Will you make the changes as
per the code block I included in my last comment?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ATEw0WruWbikGpW8-2WMAYo9ip8RQRSwks5qijDhgaJpZM4Joxhf
.

@kmckinnis
Copy link
Contributor Author

Also uuid.UUID causes a failure as uuid is not defined

On Tue, Aug 23, 2016 at 9:56 AM, Kevin McKinnis kmckinnis@biota.com wrote:

Sorry I didn't see that part of the email. Didn't hit read more.

On Mon, Aug 22, 2016 at 4:29 PM, John Chase notifications@github.com
wrote:

@kmckinnis https://github.com/kmckinnis Will you make the changes as
per the code block I included in my last comment?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ATEw0WruWbikGpW8-2WMAYo9ip8RQRSwks5qijDhgaJpZM4Joxhf
.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.407% when pulling e0f3859 on kmckinnis:master into 1bb93ae on johnchase:master.

@johnchase johnchase merged commit 864da29 into johnchase:master Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants