search

shopspring / decimal

Arbitrary-precision fixed-point decimal numbers in Go
Other
6.29k stars 620 forks source link

Consider linking project fork: greatcloak/decimal #324

Open epelc opened 1 year ago

epelc commented 1 year ago

Hello,

Not sure if anyone is still around to add a link to readme. We just started a fork after using decimal for a long while.

Sadly it seems shopspring was bought out and no longer maintains this repo. Our company develops ecommerce and shipping software and is starting a fork. We use this project as part of our billing system so have a vested interesting in maintaining it and fixing any bugs.

We've just launched version 1.4.1 which attempts to cleanup the project, fix linter warnings, reorganize/split files into more readable ones, etc. We've also added BSON support and made it easier to work with 0 values for JSON by accepting an empty string when unmarshalling.

https://github.com/greatcloak/decimal

Our future plans are to look at PRs for bug fixes such as https://github.com/shopspring/decimal/pull/322, https://github.com/shopspring/decimal/pull/301, and others. We are a bit more open about adding 3rd party deps where it makes sense. However we are using this in several products so also value non breaking changes and compatibility much like shopspring.

temuera commented 1 year ago

this is a good news!

varfrog commented 1 year ago

Good news, so cool to see abandoned projects revived. Could you please add the latest release tag in that new repo? Currently it shows the number of tags but not the latest release

epelc commented 1 year ago

@varfrog here you go https://github.com/greatcloak/decimal/releases/tag/v1.4.1

harryzcy commented 1 year ago

@epelc Can you please enable issues on your repo?

epelc commented 1 year ago

It is enabled now

mwoss commented 9 months ago

Hi all! Sorry for the late reply :<, I was busy working on other projects and was not able to take care of this library. Priorities changed now and I should be able to put some time and love into this library :3 (at least to fix some crucial bugs)

I can mention your project in the documentation but I want to understand what's your vision on this fork. I saw the project was 8 commits ahead, but most of them were about file reorganization and linting. The only new thing is support for BSON marshaling that we don't want to implement as we are aiming for a zero-dependency library - see https://github.com/shopspring/decimal/pull/92 and https://github.com/shopspring/decimal/issues/168. Are you aiming to support multiple different DB drivers? What would be the main difference between this library and your fork?

epelc commented 9 months ago

Hi @mwoss thanks for getting back. Curious what you are working on now. Sounds exciting! Thanks for spurring some action here as well I've just applied a couple bug fixes for open PRs here

Anyways we will keep our fork going since many of our projects depend on it and our main priority is to make web application development safer/easier.

Our main priorities:

I agree with you that keeping the library dependency free is a major positive. However I think there can be a caveat that core features/majority of the library is low/dependency free while sprinkling in some dependencies where appropriate is helpful. In this case making the Decimal data type work with common databases by default is very helpful in regular apps and tests.

Auto register/builtin marshalers/unmarshalers vs explicit setup

One could argue that the decimal_bson.go could be moved to a subpackage and even a separate go module. However this would require users to manually call RegisterBSONDecimalCodec in all projects. The more important issue is that you also have to do this in all tests as well which is the more common place that people forget.

mwoss commented 9 months ago

Thanks for such a detailed rationale @epelc :3 So the main difference would be a support for various DB drivers that could allow easier web application development, and the company support itself. Makes sense to me :3 The last bullet point applies to both this library and your fork in my opinion (at least right now).

I would do a small research after New Year's Day and gather all forks and library alternatives that seem like good candidates to be mentioned in the README. I'll let you know when PR with this change will be ready. Have a great new year and new eve! :D

mwoss commented 5 months ago

Hi @epelc! I'm sorry it took me so long to do this, I had a lot of things on my head in the last few months. Here's the PR https://github.com/shopspring/decimal/pull/363 that introduces a new section to the docs (alternative libraries) when I briefly mention your fork and other similar libraries. Please take a look if this is fine for you :D

One more small thing. I saw that instead of cherry-picking you copy pasted a few commits to your fork. I would highly appreciate it if in the future you would only use git cherry-pick to keep the fork in sync with the origin repo. This repository is under the MIT license and you can do whatever you want with it, but I believe original authors would be happy if they get some credits for their work. I hope you understand my point of view :D

epelc commented 5 months ago

Thanks @mwoss! Appreciate the link and will try to git cherry-pick in the future. Sorry about that was a new workflow. It was my intent to keep their credits and authorship.

I think I failed originally on the first couple merges but got it right on the two latest ones. I'll definitely confirm going forward.

image

New w/ credits on later ones. I need to look in going forward git cherry-pick though as I'm not 100% this keeps the forks in sync like you said even though it has attribution working. I'll double check next time I merge in any changes.

image

I looked at the PR and it looks good. Seems like we are the primary fork focusing on keeping the library's api as close to as it was. We are focusing on keeping arbitrary precision support as our use cases are billing and ecommerce related web applications. Anything good for web apps and websocket/rest json apis is a plus for us. Long term stability and bug fixes are also a major priority due to our use in several large applications.

epelc commented 5 months ago

@mwoss released an update w/ latest perf and bug fixes using git cherry-pick. Thanks for the info about git!

https://github.com/greatcloak/decimal/compare/v1.4.3...v1.4.4

mwoss commented 5 months ago

Thanks for doing that, I greatly appreciate that :D

Should I add mainly focused on solving billing and e-commerce related problems next to your library in mentioned PR? What do you thin?

epelc commented 5 months ago

How about something like this? I think web applications is a good keyword since that is a common use case people are looking for and is what we are primarily using for.

fork of this library, with out-of-the-box BSON marshaling support -> fork focusing on billing and e-commerce web application related use cases. Includes out-of-the-box BSON marshaling support

mwoss commented 5 months ago

Sounds great to me, I will use that :3 Thanks!