Re: [jgit-dev] Is there a roadmap for JGit 5.0?

[Marc Strapetz <marc.strapetz@xxxxxxxxxxx>]

On 26.10.2017 10:34, Marc Strapetz wrote:
> On 25.10.2017 23:16, Matthias Sohn wrote:
> > On Wed, Oct 25, 2017 at 2:55 PM, Marc Strapetz 
> > <marc.strapetz@xxxxxxxxxxx <mailto:marc.strapetz@xxxxxxxxxxx>> wrote:
> >
> >     I'm asking because as I understand for version 5.0 API changes would
> >     be allowed? If so, I'd like to bring up an older topic once again 
> > [1]:
> >
> >     Config methods should throw checked Exceptions instead of
> >     IllegalArgumentExceptions in case of invalid values: a user can
> >     easily introduce invalid values into .git/config. This does not
> >     represent an internal or API usage error. The current API, which is
> >     using IllegalArgumentException, makes it very hard to detect and
> >     handle such cases. Once the time is right, I can provide a (huge)
> >     patch here, too, which I'm maintaining since a couple of years.
> >
> >     [1] https://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02529.html
> >     <https://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02529.html>
> >
> >
> > I added your proposal to the 5.0 plan here
> > https://projects.eclipse.org/projects/technology.jgit/releases/5.0.0/plan 
> > <https://projects.eclipse.org/projects/technology.jgit/releases/5.0.0/plan> 
> >
> >
> > I propose we start working on 5.0 after we released 4.10 in December.
>
> Thanks! I'll watch out for the 4.10 release and will cleanup my patch 
> then and upload.

I've now uploaded a draft addressing this change:

First, it's convenient to derive ConfigInvalidException from 
IOException; at least in JGit the handling of both types was almost 
always identical:

https://git.eclipse.org/r/#/c/119969/

The main patch introduces a new ConfigIllegalValueException, derived 
from ConfigInvalidException:

https://git.eclipse.org/r/#/c/119970/

For this first draft, ConfigIllegalValueException is only used in
DefaultTypedConfigGetter#getBoolean for now. The Exception is propagated 
upwards until affected methods provide other declared Exceptions for 
which it's reasonable to have them wrapping ConfigIllegalValueException, 
this is especially GitAPIException and TransportException.


What's I'm currently not sure about:

- whether we need a separate class ConfigIllegalValueException? As long 
as ConfigInvalidException is derived from IOException, it could easily 
be substituted. On the other hand, when strictly using 
ConfigIllegalValueException for invalid key-values only, it's more 
informative than a generic ConfigInvalidException. It also might have 
addition key value parameters which carry the invalid values.

- whether it's (perfectly) reasonable for TransportException to wrap 
ConfigIllegalValueException? It's definitely convenient to do so.

- do we need detailed Javadoc @throws explanation everywhere where 
ConfigIllegalValueException is used?


Left to do:

- Use ConfigIllegalValueException instead of IllegalArgumentException 
for other occurrences in DefaultTypedConfigGetter (and maybe other places)

- Fix Javadocs

Also, how can I invoke Jenkins on my drafts? Adding "CI Bot" reviewer 
didn't work (at least no job has been started after 30 minutes).

-Marc