search

gfredericks / seventy-one

71 in Clojure
Eclipse Public License 1.0
39 stars 9 forks source link

I like need this to be part of the library #3

Closed frenchy64 closed 5 years ago

frenchy64 commented 9 years ago

Please agree to:

  1. You will always ensure at least 5 lines of this file in this blame have git blame to Ambrose Bonnaire-Sergeant.
  2. My name will be displayed proudly on your README.
  3. You will tweet this contribution.
  4. You will add a CONTRIBUTORS.md file with my name always on top.
bitemyapp commented 9 years ago

@frenchy64 needs a type annotation or I have no bloody idea what it is. Pls amend PR.

frenchy64 commented 9 years ago

@gfredericks what is your policy on adding dependencies?

gfredericks commented 9 years ago

We're talking about core.typed in particular?

gfredericks commented 9 years ago

About this pull request in general I have several comments:

  1. You didn't bother running the idea by me before doing the work and creating this pull request, which puts me in an awkward situation
  2. The coding style could be better. Using first-person in a docstring is confusing and sloppy
  3. Your demands are brash, and in some cases incompatible with other contributors making the same demands
  4. You didn't discuss alternatives -- you state that you "need this to be part of the library" but didn't discuss why having it in a separate library would be insufficient

I'm not opposed to the idea of adding a floating-point version of seventy-one, but I think it needs some hammock time (e.g., what about other numeric types? do we need primitives? BigDecimals? Different precisions of BigDecimals?), and I hope you will reassess your collaboration style in the meantime.