Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] [VOTE] Making API incompatible changes

That doesn't sound right to me. IMO it's time for some semantic versioning.
http://www.osgi.org/wiki/uploads/Links/SemanticVersioning.pdf

The API you mention sounds like a service provider API, i.e. it's used to provide additional services to JGit. The proposed API change is backwards compatible for consumers of the API. It is a breaking change for providers of the API. According to the semantic versioning scheme, this requires a bump up of the minor version part only.

Thus, my proposal for this kind of change is:
  3.0 -> 3.1

-Gunnar

-- 
Gunnar Wagenknecht





Am 21.06.2013 um 16:09 schrieb Shawn Pearce <spearce@xxxxxxxxxxx>:

https://git.eclipse.org/r/12422 makes an incompatible API change by
adding a new abstract method to ObjectInserter.

Since its an API break, we would need to bump the next version to 4.0.
However master will deliver to the Kepler stream until next June.
Which means we need to wait 1 year to make this change.

I'm not going to wait a year to add a method to an abstract class that
99.99999% of consuming applications will never extend. I see a few
options:

Option 1)  Google forks JGit.

 We fork. We host the fork at gerrit-review. The fork has the change.
 Our development work continues at gerrit-review.

Option 2)  Bump version to 4.0

 Master jumps to 4.0. We (maybe) keep a kepler branch that will be
 used to deliver to the kepler stream through next June.

Option 3)  Concrete method throws UnsupportedOperationException

 Proposed in comments. The new method throws an exception unless
 the subclass "correctly" knows it should override the method with an
 implementation. To an API checking utility its not an API breakage,
 but now implementors (which should be 0!) do not know they need to
 override the method for correct behavior from JGit.

Option 4)  Stupid API workarounds

 Instead of saying inserter.newReader() we implement a bridge:

   class ObjectReader {
     public static ObjectReader newReader(ObjectInserter ins) {
       if (ins instanceof ObjectInserter2) {
         return ((ObjectInserter2)ins).newReader();
       }
       throw new UnsupportedOperationException();
     }
   }

   package org.eclipse.jgit.lib.internal;

   abstract class ObjectInserter2 extends ObjectInserter

 Like option 3 implementors of ObjectInserter do not know they
 need to implement ObjectInserter2. But now we have a pattern
 we can use to make additional API changes to ObjectInserter,
 if they ever arise.

 I hate this option because it hides a useful function of one type
 (ObjectInserter) inside of a static method of another type. Bleh.
 Another place might be to try and do this on Repository, but its
 the same sort of ugly.


12422 is needed to improve write performance or our hosted Gerrit
solution on a JGit DFS backend. Since Dave Borowitz has most of the
work done we don't want to wait until next June to land the changes.

Similarly Jonathan Neider wants to do some refactoring on the new
archive implementations to support additional use cases, like
combining multiple repositories into a single archive stream, or
annotating archives with their MIME types. So this issue of API
breaking changes is going to crop up again within the next 12 months.
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/jgit-dev


Back to the top