Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] Is there a roadmap for JGit 5.0?

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










Back to the top