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

lib/bn.js is too big #35

Closed
dcousens opened this issue Feb 12, 2015 · 13 comments
Closed

lib/bn.js is too big #35

dcousens opened this issue Feb 12, 2015 · 13 comments
Assignees

Comments

@dcousens
Copy link
Contributor

With a SLOC count of over 2000 for a single file.
Perhaps its time to break out parts of it?

@dcousens dcousens changed the title lib/bn.js is getting larger lib/bn.js is bigger Feb 12, 2015
@indutny
Copy link
Owner

indutny commented Feb 16, 2015

What suggestions do you have? The nice thing about this project is that you could just use lib/bn.js in a browser.

@dcousens
Copy link
Contributor Author

I only have suggestions that compromise that capability.

@chevdor
Copy link

chevdor commented Mar 7, 2015

I would suggest to provide a minified version. That would reduce the amount a bit but I don´t see it much as an issue.

@dcousens
Copy link
Contributor Author

@chevdor my concern isn't size in part of distribution or use in production.

My concerns were primarily in being able to review and audit a project which consists entirely of a 2000+ SLOC file.
In any other language, this could [probably] be considered a failure in the projects architecture.

@indutny
Copy link
Owner

indutny commented Mar 26, 2015

After some consideration: I don't mind if it'll be split into the multiple files, provided that there will be a all-included js distribution, and the way to include all plugins would be simple/transparent for users.

@dcousens any suggestions how to do it?

@dcousens
Copy link
Contributor Author

dcousens commented Apr 7, 2015

I think just a flat file structure based on the different context classes
should be enough?
On 27 Mar 2015 4:32 am, "Fedor Indutny" notifications@github.com wrote:

After some consideration: I don't mind if it'll be split into the multiple
files, provided that there will be a all-included js distribution, and the
way to include all plugins would be simple/transparent for users.

@dcousens https://github.com/dcousens any suggestions how to do it?


Reply to this email directly or view it on GitHub
#35 (comment).

@indutny
Copy link
Owner

indutny commented Apr 7, 2015

Yeah, this sounds awesome. Does it sound like an interesting project to you? ;)

@dcousens
Copy link
Contributor Author

dcousens commented Apr 8, 2015

Sure, I'll wait for you to merge #27 to save me an annoying rebase/merge effort.

@dcousens
Copy link
Contributor Author

dcousens commented Apr 8, 2015

Its a shame you can't assign issues to people involved in the conversation.

@dcousens dcousens self-assigned this Apr 16, 2015
@dcousens dcousens changed the title lib/bn.js is bigger lib/bn.js is too big Sep 20, 2015
@jprichardson
Copy link
Collaborator

@dcousens #27 has been merged 😜

@indutny
Copy link
Owner

indutny commented Nov 29, 2015

This PR was hanging this for one year! :)

@dcousens
Copy link
Contributor Author

Haha. I'll get around to this some time soon then 👍

@dcousens
Copy link
Contributor Author

dcousens commented Oct 25, 2016

I suspect this is probably never going to be worth the time considering the relative stability of this code now.

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

No branches or pull requests

4 participants