-
Notifications
You must be signed in to change notification settings - Fork 187
Extract createAssetStringID
#1229
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
Extract createAssetStringID
#1229
Conversation
This reverts commit 2c20a59.
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is doing too many things at once, making it difficult to review.
Please do the indentation changes in a later PR and only extract the helper function here.
Github is bad at displaying changes if you change the indentation and move code around. So please do them separately. (Or at least make separate commits, so I can look at them separately, but I've told you this already)
|
Here. Since the code was duplicated for |
createAssetStringID from readAllZonFilesInAddonscreateAssetStringID
|
Ok. I am refusing to figure out every possible edge case that is a result of this design decision. |
|
All I said is that you should print an error if there is a dot before the extension (or only remove the extension, as it was before). Everything else is outside the scope of this PR anyways. |
|
Here, for ".zig.zon" there is special case that removes it, for everything else there is generalized removal of last suffix, hopefully I didn't mess up the indexes. idk, they don't like me and I don't like them. |
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's better, thank you.
(Sadly didn't save any lines, but we'll get there with the next read*Files function)
Extracted from #1207