Skip to content

[WIP] make js / d.ts files consistent #735

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
wants to merge 1 commit into from

Conversation

sly7-7
Copy link
Contributor

@sly7-7 sly7-7 commented Feb 7, 2025

It's a WIP related #715

As per #715 (comment) , I've removed all the FileTypeOptions arguments passed in helper functions fileTypeFromXXX.
Some tests are now failing, it emphasizes the fact that some functions currently have the ability to take a FileTypeOptions argument but not others.

I've also reorganized a bit the file structure of core.js/core.d.ts and index.js/index.d.ts to make them more or less aligned.

Please let me know if that makes sense, and what would you do about the "helpers" functions, make all of them or none of them able to take FileTypeOptions argument.

@sly7-7 sly7-7 changed the title make js / d.ts files consistent [WIP] make js / d.ts files consistent Feb 7, 2025
@Borewit Borewit marked this pull request as draft February 19, 2025 16:36
@Borewit
Copy link
Collaborator

Borewit commented May 22, 2025

@sly7-7 I think I have addressed a number of inconsistencies in #752. Do you mind review / rebase or close this one?

@Borewit
Copy link
Collaborator

Borewit commented Jun 8, 2025

Considered to be abandoned.

@Borewit Borewit closed this Jun 8, 2025
@sly7-7
Copy link
Contributor Author

sly7-7 commented Jun 11, 2025

@Borewit Hi, I'm sorry for the lack of feedback, I was in a hurry for my company's project 😒 , and didn't had the time to review. I've just checked what you did, and it seems to me you finally choose the opposite solution you exposed here, but anyway the main thing is that all look consistent now. Thank you 🙏

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