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

Allow uppercase letters in flag keys #172

Closed

Conversation

kalebdf
Copy link

@kalebdf kalebdf commented Oct 6, 2018

Flag keys may now allow uppercase letters.

Description

This PR allows Flag keys to be alphanumeric & case insensitive as well as include underscores. It does remove the constraint of having at least one lowercase letter as the first character.

Motivation and Context

The first FLAG in the Flagr demo had a flag key (e.g. kppaP9af9x10) which had an uppercase letter which threw an error when I tried to change the description because of the capital letters in the flag key.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@zhouzhuojie
Copy link
Collaborator

Thanks for the PR, I think I will fix the demo sqlite db, not the valid key regex.

@kalebdf
Copy link
Author

kalebdf commented Oct 6, 2018

Thanks for the PR, I think I will fix the demo sqlite db, not the valid key regex.

@zhouzhuojie, Okay, thanks! 👍

I wasn't quite sure how that DB was updated or I would have done that 😄. Is it auto-generated from the API /export/sqlite or do you edit the row and then re-commit it? Also, is there a reason for only allowing lowercase characters + numbers + underscores in keys?

@zhouzhuojie
Copy link
Collaborator

The demo db was fixed in #173.
So https://try-flagr.herokuapp.com should be good to go now.

Yes, you can just run /export/sqlite, or start a new one with env DBConnectionStr points to it, it's basically a binary file.

Also, is there a reason for only allowing lowercase characters + numbers + underscores in keys?

I don't have a strong reason, it was used to limit the charset of variant key at the beginning. Keys can be changed by users (e.g. Variant Key, Flag Key), and without uppercase it's just easier to search.

@kalebdf
Copy link
Author

kalebdf commented Oct 6, 2018

Makes sense. Thanks @zhouzhoujie.

@kalebdf kalebdf closed this Oct 6, 2018
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