| Re: [jgit-dev] Is there a roadmap for JGit 5.0? |
On 22.03.2018 15:11, David Pursehouse wrote:
On Thu, 22 Mar 2018, 22:55 Marc Strapetz, <marc.strapetz@xxxxxxxxxxx <mailto:marc.strapetz@syntevo.com >> wrote:
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@syntevo.com >
<mailto:marc.strapetz@syntevo.com
<mailto:marc.strapetz@syntevo.com >>> 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/releas es/5.0.0/plan
>>
<https://projects.eclipse.org/projects/technology.jgit/relea >ses/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.
Is there a way for me (normal user) to manually trigger? This would also be helpful for regular changes for which Jenkins sometimes fails (for unknown reasons).
Also, since the changes are draft, only people who have been explicitly added as reviewer can see them.
Thanks, I didn't know. So I guess I should convert it to a normal change immediately, otherwise no one of you can have a look to give feedback? Or do committers still have access?
-Marc