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

travis: reject invalid UUIDs #862

Merged
merged 3 commits into from
Aug 31, 2019
Merged

travis: reject invalid UUIDs #862

merged 3 commits into from
Aug 31, 2019

Conversation

petertseng
Copy link
Member

Want to prevent any invalid UUIDs from being entered.

Want to put this in configlet lint, but can't:
exercism/configlet#99
exercism/configlet#168
So it will go in individual tracks' .travis.yml for now.

It appears that the site will accept pretty much arbitrary strings as
UUIDs for now, but we want to make less work for ourselves when valid
UUIDs are required.

Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Changes look good, and I see from the commit history that this does what it's meant to.

I have a low-strength preference that this feature be moved from .travis.yml into a new script file in _test, for consistency with the other tests, and so that it can be run independently. This preference is not strong enough to block merging as-is.

@petertseng
Copy link
Member Author

It shall be done (moved into a script)

Want to prevent any invalid UUIDs from being entered.

Want to put this in configlet lint, but can't:
exercism/configlet#99
exercism/configlet#168
So it will go in individual tracks' .travis.yml for now.

It appears that the site will accept pretty much arbitrary strings as
UUIDs for now, but we want to make less work for ourselves when valid
UUIDs are required.
too short, too long, non-hex char
@petertseng
Copy link
Member Author

I have pretty high confidence in the tests, enough that I would consider just merging it now, but I'll leave it open for about 16 hours in case additional comments pop up.

@petertseng petertseng merged commit e79e43f into exercism:master Aug 31, 2019
@petertseng petertseng deleted the uuid branch August 31, 2019 05:08
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.

2 participants