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

Converted to TypeScript #385

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Domratchev
Copy link

Upgraded tools: Webpack 4.38, Babel 7.5, Karma 4.2, Chai 4.2, Sinon 7.3
Removed redundant files
Removed all 3rd party dependencies for Web version
Changed ImageDebug API
Changed Box interface to be [Point, Point, Point, Point]
Improved performance by utilizing ES6+ features, bitwise operators, etc.
Added support for area measured in pixels (px)

Upgraded tools: Webpack 4.38, Babel 7.5, Karma 4.2, Chai 4.2, Sinon 7.3
Removed redundant files
Removed all 3rd party dependencies for Web version
Changed ImageDebug API
Changed Box interface to be [Point, Point, Point, Point]
Improved performance by utilizing ES6+ features, bitwise operators, etc.
Added support for area measured in pixels (px)
@Domratchev Domratchev mentioned this pull request Aug 2, 2019
@ericblade
Copy link
Collaborator

@Domratchev wow! This is obviously a massive change, which I'm very grateful for, but I'm also not wanting to jam this through in serratus's repo, while this is all still in flux (witness the "is quagga dead" thread :( )

Please hang onto this, I'll be trying to setup a release setup for a fork over the next week or so, and I'd be absolutely thrilled to have such an update for it.

@ericblade
Copy link
Collaborator

Hi @Domratchev I've setup a fork at https://github.com/ericblade/quagga2 that I intend to maintain, and I'd love to have your contributions there.

@ericblade
Copy link
Collaborator

@Domratchev I've been looking through this change, and although I do think that it's quite good, I'm also noticing that it's a single commit, that has a lot of changes that aren't exactly 100% relevant to converting to typescript -- and while it's work that needs to be done, cleaning up the outdated bits, modernizing, streamlining, etc -- that sort of thing really should be handled in separate commits from the typescript conversion.

I've already performed some of the things in this patch that aren't related to typescript in my fork, and I'm definitely picking through your change to find out other things that you've done here -- because I think what you've done here is great.

Ultimately, though, I'd like to see multiple commits that do the things unrelated to typescript, and then a commit that enables typescript, and probably multiple commits that actually implement it. With this one-commit-does-everything approach, if there's any breakage at all, it could be quite a bit more difficult to track down where/what created the problem.

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