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

Bugfix: fail at getting uuid more gracefully #1702

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Conversation

mixmix
Copy link
Member

@mixmix mixmix commented Apr 25, 2023

was seeing CI fail on this particular line in another PR, this is an attempt to fix that

@@ -77,8 +77,12 @@ const getCard = (cardCode) => {

const getUuid = (cardCode) => {
const [setCode, cardNumber] = cardCode.split(":");
const { cardsByNumber } = getSet(setCode.toUpperCase());
Copy link
Member Author

Choose a reason for hiding this comment

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

this line here was the gotcha - destructuring undefined was causing an error to be thrown in CI

I don't know why the set would not be found, maybe that's an error which should throw, as it shows your data is somehow really wrong.

@mixmix
Copy link
Member Author

mixmix commented Apr 25, 2023

Node 18 failed - didn't download the data needed. Re-running to see if it passes

@mixmix
Copy link
Member Author

mixmix commented Apr 26, 2023

WIP - closest I've got is that there is something going wrong in Node 18 CI around download of sets - it seems like malformed sets are being saved e.g. data/sets/MOM.json is invalid JSON.

@mixmix mixmix changed the title bugfix - fail at getting uuid more gracefully WIP: bugfix - fail at getting uuid more gracefully Apr 26, 2023
@tooomm tooomm requested a review from ZeldaZach October 29, 2023 09:22
@tooomm tooomm marked this pull request as draft October 29, 2023 09:23
@tooomm tooomm changed the title WIP: bugfix - fail at getting uuid more gracefully Bugfix: fail at getting uuid more gracefully Oct 29, 2023
@tooomm tooomm mentioned this pull request Oct 29, 2023
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