commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Raymond DeCampo <...@decampo.org>
Subject Re: [Numbers] Further work on NUMBERS-6 (Was: [5/5] Add maven module for fractions [...])
Date Sat, 28 Jan 2017 19:38:21 GMT
On Sat, Jan 28, 2017 at 10:48 AM, Gilles <gilles@harfang.homelinux.org>
wrote:

> On Sat, 28 Jan 2017 09:00:52 -0500, Raymond DeCampo wrote:
>
>> Thanks for the feedback Gilles.
>>
>
> Thanks for helping with the project!
>
> I generally prefer to work in smaller increments so that suits me fine.
>>
>> Re working on an outdated branch, I'm not sure I completely understand
>> what
>> you want plus I am not a git expert by any stretch.
>>
>
> I'm not either.
> Hence I prefer to keep it simple and avoid potential conflicts by creating
> a branch from master HEAD and do as many "git rebase" as necessary to keep
> in sync with master while working on "feature X".
>
> Are you suggesting I
>> back out the changes on the multimodule branch using rebase?
>>
>
> Not sure that's the easiest way.
>
> I've just created a
>   fraction__NUMBERS-6
> branch based on your work, and did
>   git rebase master
> on it.
>
> Let's now work on that one (and delete "multimodule") until we are happy
> about its contents.
>

Sounds like a reasonable plan.


>
> Then
>> following the guidelines from doc/development create a feature branch and
>> then chunk the changes up into digestible commits on that branch?
>>
>
> Not worth the time, now.
> I mentioned it for future work; when commits are large, people tend to
> not want to review them. ;-)
>
> I realized my mistake on not referring to the JIRA issue on the commit
>> right away (it's always something).
>>
>
> I fixed that in "fraction__NUMBERS-6", with
>   git commit --amend
>
> Now, for the contents of the "fraction" module.
>
> (1)
> My main current concern is the formatting-related classes; as for
> "complex", I think that
>  1. formatting (and I/O) is out of scope, and
>  2. the implementation were problematic in CM already.
> Hence, I'd prefer to drop support altogether.[1]
>
>
I see your point of view, but allow me to play devil's advocate.  It seems
natural to create the formatter for a new object, in particular a number
type, close to the type itself.  Making new objects and expecting another
project to implement the formatting seems like what economists call an
externality.  I suppose it really depends how everyone maintaining text
would feel about it.  I am somewhat concerned about how the release cycle
would work, in particular for the initial released, where text would have
to wait for a release of numbers before including the formatting and then
users would not have formatting until the next release of text.

That said if the people maintaining text agree I have no objection.


> (2)
> Another concern is what to do with the "XxxField" classes.
>

Do we have any idea if the user base uses these classes?  It's no problem
to continue to maintain them (at least from my point of view) but I am
having difficulty imagining the use case.


>
> (3)
> All "@since" tags must be removed.
>
> (4)
> Right now, I think that we should not advertize specific exceptions.
> I.e. they should have at most "package" visibility, or maybe even
> better: private inner class.
> The Javadoc "@throws" should mention the JDK's parent class.
> This is a departure from what was done in CM, where the design was
> trying to satisfy mixed requirements, never shown (IMO to be really
> necessary for a supposedly "low-level" library).
>

That suits me fine.


> My stance is that whenever an exception is thrown from an application,
> there is a programming error that must be solved by looking at the
> code (to find the faulty call or bug in the library).
>
Regards,
> Gilles
>
> [1] There could be a "math-module" in "Commons Text" that would do
>     such things, with the support of more advanced text manipulation
>     classes.
>
>
>>
>>
>> On Sat, Jan 28, 2017 at 8:36 AM, Gilles <gilles@harfang.homelinux.org>
>> wrote:
>>
>> Hi Ray.
>>>
>>> On Sat, 28 Jan 2017 12:44:27 -0000, raydecampo@apache.org wrote:
>>>
>>> Add maven module for fractions package from commons-math
>>>> Resolves pull request #4
>>>>
>>>>
>>> Whenever possible, we aim at small commits (unless they are all of
>>> the same kind, and it is obvious what is being done repeatedly).
>>>
>>> In particular, independent codes should be added incrementally, to
>>> allow for easier review.
>>> For example, modifications to "pom.xml", addition of "ArithmeticUtils",
>>> "Fraction" and "BigFraction", etc.
>>>
>>> Please note that you worked on an outdated branch.
>>> [Please read the guidelines in "doc/development".]
>>> Can you fix that (using "rebase" I guess) before we discuss the
>>> contents of the commit?
>>>
>>> Also to allow easy review: the commit message should refer to the
>>> JIRA issue identifier).
>>>
>>>
>>> Thanks,
>>> Gilles
>>>
>>>
>>>
>>> [...]
>>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message