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

Add .internal file handling from the setup repo #16

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

Conversation

Mevas
Copy link

@Mevas Mevas commented Aug 31, 2022

Refactored the way data urls are obtained and added node-fetch to have access to the status of the request, so that the .internal version of a data file is tried in the case of the initial request returning a 404.

The PR is marked as draft as there seem to be some differences in the format of internal files compared to what normal files looked like, I assume, leading to the majority of the tests breaking, such as there being no "IJN" nationality, instead expecting "Sakura Empire". The data format is outside the scope of my knowledge, so someone else should most likely fix these compatibility issues.

Also refactor how data urls are obtained
@jasperchua99
Copy link
Member

would recommend yea to split the node fetch to a separate pull request :)

@0t4u
Copy link
Collaborator

0t4u commented Oct 28, 2022

The .internal files are marked for further transforming IIRC. I'm not sure if we should use them because that is not a fully transformed file. Probably needs additional discussion with @octo-kumo, who is the maintainer of the setup repo.

@0t4u
Copy link
Collaborator

0t4u commented Oct 28, 2022

Either that or they are just marked as internal because it is "unstable". I have no clue tbh.

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.

3 participants