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

Remove getId call from set id generation #323

Merged
merged 3 commits into from
Mar 26, 2024
Merged

Remove getId call from set id generation #323

merged 3 commits into from
Mar 26, 2024

Conversation

JakeWags
Copy link
Member

Does this PR close any open issues?

Closes #322

Give a longer description of what this PR addresses and why it's needed

This PR removes the getId call from the set's id generation to remove any interference with existing set names. The _ filtering isn't being used anywhere, and so this can be entirely removed.

Provide pictures/videos of the behavior before and after these changes (optional)

Old alt-text:

image

Updated alt-text:

image

Old data-table (visible sets):

image

Update data-table (visible sets):

image

Have you added or updated relevant tests?

  • Yes
  • No changes are needed

Have you added or updated relevant documentation?

  • Yes
  • No changes are needed

Are there any additional TODOs before this PR is ready to go?

TODOs:

  • None

Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for upset2 ready!

Name Link
🔨 Latest commit 027adf4
🔍 Latest deploy log https://app.netlify.com/sites/upset2/deploys/6601b986c8e5b100083680e5
😎 Deploy Preview https://deploy-preview-323--upset2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@JakeWags JakeWags requested a review from NateLanza March 25, 2024 17:44
Copy link
Contributor

@NateLanza NateLanza left a comment

Choose a reason for hiding this comment

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

LGTM

@JakeWags JakeWags merged commit 40a1573 into main Mar 26, 2024
7 checks passed
@JakeWags JakeWags deleted the 322-set-naming branch March 26, 2024 21:24
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.

Sets with spaces are incorrectly named for alt-txt/grammar reasons
2 participants