Skip to content

Improve documentation #32

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

Merged
merged 3 commits into from
Oct 6, 2020
Merged

Conversation

pietroppeter
Copy link
Contributor

Hi,
another small incremental improvement for documentation:

  • added a short summary of package features
  • added a list of current limitations and potential enhancements (taken from TODO list and issues)

There is one element of TODO list that I think it should not be an issue anymore (since there are no more nil for sequences) and so I did not list it in current limitations: "Fix the templates so they work with uninitialized (nil) values!"

Let me know what you think.

Regarding possible API changes in the future I have always wondered why Negative is an enum instead of a bool (where there other flags at the beginning?), but for the moment this is just curiosity, plan is still to fix outstanding issues before thinking of the improvements. I am enjoying the process and learning quite a bit, the code is very terse.

In the meantime I have started to work on #26 (see https://github.com/pietroppeter/nim-bigints/tree/fix-empty-limbs). I think there are many places where empty limbs might raise an issue, the plan is map them feature by feature with failing tests and fix them one by one. I will open a draft PR when I complete fixes for the compare operators and proceed from there.

Also notable is a new bigints library that was born a couple of days ago and which probably aims to have same footprint as bigints (I do expect it to progress fast and be very performant given its author): https://github.com/SciNim/megalo/tree/master/megalo

@def-
Copy link
Member

def- commented Oct 6, 2020

Hm, so are you still planning to work on this? I think it would make more sense to combine the efforts in one project, especially considering NIm's community size. mratsim probably had good reasons to rewrite instead of adapting my code, so I'd talk to him if I were you. If megalo will be better than nim-bigints and a drop-in-replacement, we could add a big link to it in the readme.

@def- def- merged commit c6d7229 into nim-lang:master Oct 6, 2020
@pietroppeter
Copy link
Contributor Author

pietroppeter commented Oct 6, 2020

well, yeah, for the moment I still plan to work on bigints. It is a good and simple library and it seems to me it just needs a bit of effort to be more "finished". I agree that it will make sense eventually to discuss and possibly join forces, but for the moment bigints is here and megalo has just started (and I guess it is probably more difficult to contribute to it in this initial phase). Maybe this will change in a week or so. I do think the effort I put in bigints right now (which is mostly learning on my side), will be still useful to possibly contributing to megalo in the future or discuss with mratsim on what are the best options for these two libraries and for optimizing effort.

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