Skip to content

fix(#113): Adds regex to random.word to trim unwanted characters #218

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

Closed

Conversation

prisis
Copy link
Contributor

@prisis prisis commented Jan 18, 2022

The rebase on #115 didnt work...

@netlify
Copy link

netlify bot commented Jan 18, 2022

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: cee82f2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61e8394966b83e00084ebed9

😎 Browse the preview: https://deploy-preview-218--vigilant-wescoff-04e480.netlify.app

@prisis prisis added the has workaround Workaround provided or linked label Jan 18, 2022
MilosPaunovic
MilosPaunovic previously approved these changes Jan 18, 2022
@damienwebdev
Copy link
Member

I still don't like this implementation. It feels kludgy. I would prefer a separate dataset rather than a regex.

@Shinigami92 Shinigami92 linked an issue Jan 18, 2022 that may be closed by this pull request
@prisis prisis force-pushed the feature/sanitize-random-word2 branch from 9702e32 to cee82f2 Compare January 19, 2022 16:16
@prisis prisis requested a review from a team as a code owner January 19, 2022 16:16
@HonzaMac
Copy link
Contributor

Would this change hit performance of generating random data? I am little bit worried of calling regex like this.

@Shinigami92
Copy link
Member

I think we have now 2 different (with me 3) that don't really like this PR 😕
Maybe we should just use that regex outside the random function call on demand

@prisis prisis closed this Jan 23, 2022
@prisis prisis deleted the feature/sanitize-random-word2 branch January 23, 2022 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has workaround Workaround provided or linked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

random.word() can return undesirable non-alpha characters
5 participants