directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Karasulu <akaras...@apache.org>
Subject Re: svn commit: r491261 [1/8] - in /directory/sandbox/triplesec-jacc: ./ admin-api/ admin-api/src/main/java/org/apache/ admin-api/src/main/java/org/apache/directory/ admin-api/src/main/java/org/apache/directory/triplesec/ admin-api/src/main/java/org/apache...
Date Sun, 31 Dec 2006 03:52:59 GMT
Hiya David,

First off let me say that I'm very glad to have you on board.  I'm also 
thankful that someone is actually working on this stuff.  You're a 
better man than I :).

David Jencks wrote:
> 
> On Dec 30, 2006, at 6:35 PM, Alex Karasulu wrote:
> 
>> David,
>>
>> Changing the package names in you jacc branch is not a good idea at 
>> this point int time because it obscures the real changes to the 
>> architecture that you've made for the JACC stuff.  Also if some things 
>> are not working already with integration tests you have introduced an 
>> extra variable when trying to figure out what's going wrong.
> 
> AFAICT the integration tests work the same before and after the change: 
> they all pass when I run them one at a time in my IDE (IDEA) and mostly 
> fail running through maven on the command line.

The problem is that the integration tests were failing to begin with 
from the commandline.  This hints that something is wrong.  So this is 
not the most ideal situation :(.

> It seems to me that the main way this rename could make seeing changes 
> harder is if you try to compare sandbox/triplesec-jacc with 
> trunks/triplesec using svn diff.  

It obscures the changes you're making WRT the JACC changes so an svn 
diff will show more lines changed when we try to merge it back into the 
trunk.  That's additional clutter to go through to evaluate what you've 
done.  I would like to review your stuff before committing a merge of it 
back to the trunk.  The best way to do this for me is to do a merge then 
svn diff it to see the total net changes in one shot.

This way I can see the effects of all your commits at one time rather 
than trying to remember what those commits were across the time you have 
the JACC branch open.

However we could make that easy by
> changing the packages in trunks/triplesec.  

Then that would cause conflicts when you try to merge your branch back 
into trunks/triplesec.

If you are looking at svn
> commit messages I don't see how the change would affect you.

Sorry I was not looking at each one of your commits on your sandbox.  I 
intended to review your cumulative work before we merge it back.  Again 
the best way to do this (besides going back and reviewing each commit) 
is to just attempt a merge to trunks and svn diff it before committing.

>> Do you immediately need the package names to be o.a.d.t.xxx right now? 
>> I think this is more a finishing touch before we do a release and 
>> should itself be done in a branch.
> 
> I'd hope that it's done long before a release so any hidden problems 
> have time to surface.  

Yeap you're totally right.  I too would like to do it sooner rather than 
later but right now our main objective is not a package rename.  It's in 
trying to comprehend how to make tsec jacc compatible and to understand 
the impact of your changes in the private branch.

BTW sorry I have not had enough time to look into this and assist you as 
I was hoping too.  These darn holidays and other work related tasks have 
gotten in the way.

Since it wasn't too hard (it took about an hour
> including running the failing integration tests one by one in my IDE) I 
> don't quite see the point of doing in a branch.

Tsec has some ugly stuff in it that depends on class names being in a 
certain package that may elude the refactoring capabilities of an IDE 
(yeah even IDEA).

If some of the integration tests are failing on the command line.  Then 
this could potentially mask problems that are being introduced by 
package renames.  Why bother dealing with this additional complication?

Oh and yeah it does not need another branch but something this big is 
nice to use a quick temp branch with just in case you don't catch any 
problems or do not do it all in one commit.  The branch costs nothing, 
can be deleted after the merge back, and is short lived.

>>
>> WDYT?
> 
> I'd rather change the packages on trunks/triplesec to make direct 
> comparisons easier than undo this change.  

Yeah but that would basically prevent you from merging your branch back 
wouldn't it?

If what is now
> trunks/triplesec ever gets released (rather than say copying 
> triplesec-jacc back to trunks) it will need this change anyway, so I'd 
> rather move that code line forward as well.  If you agree I'm happy to 
> change the packages there as well.

Yes of course the package names need to be updated before a release I 
totally agree.  However we cannot apply these package renames to both 
trunks and your branch otherwise the merge back from jacc->trunks would 
produce conflicts all over.

IMO I'd leave this package renaming thing to be handled later.  There is 
no urgency with it right now and furthermore it will complicate things.

In your JACC branch I'd purely focus on the one goal of that branch.

Regards,
Alex

Mime
View raw message