[
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).
The CI is triggered on patch set uploaded event, and possibly also on draft published. It won't be triggered by adding the bot as reviewer.
Also, since the changes are draft, only people who have been explicitly added as reviewer can see them.
