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

Code refactor, fixed bugs and add Win64 func #305

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

Conversation

GermanAizek
Copy link

@richgel999, @cwoffenden review my changes. Thx.

@cwoffenden
Copy link
Contributor

@WiredGermanAizek this isn't my project. I would suggest though that since you have a lot of changes, reviewing would be easier if they were broken into individual commits, and with a description as to why you'd make that change.

Looking at your code, there's some C++ modernisation (use of default, move semantics, for example) which could offer a better codegen, but it's only worthwhile (IMO) if it brings an improvement.

As for the Win64 additions, doesn't it only make sense for larger files?

And what are the bugs addressed?

@richgel999
Copy link
Contributor

Could you describe the reasoning behind these changes? I'm pretty conservative about taking PR's. The more focused they are, the better. Thanks!

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