Re: Request for advice

[Andreas Huber <a....@corax.at>]

Hope you don't mind me, cc'ing this to dev@.

Thanks Dan, you provided great insight ...

On 06.10.2017 18:14, Dan Haywood wrote:
> [...]
>
> Right now there are three streams of work:
>
> - dev/2.0.0/ISIS-1276-dn-5-1 ... moving up to DN 5.1 was the original 
> intention of 1.16.0.  However, per wiki page [6], it seems that 
> there's no objection to making this part of 2.0.0, so that's what I'm 
> aiming for.
>
> - dev/2.0.0/ISIS-1742-remove-deprecations ... I started to do this on 
> top of the DN 5.1, then decided to split it out.  That's because I'm 
> hoping to run our app, Estatio on it, see whether there are any 
> performance gains, but without the "risk" of also moving up to DN 
> 5.1.  Once all the deprecated stuff is removed, I'll be looking to 
> merge the DN 5.1 stuff on top of it.
>
> - master which is for maintenance of 1.15.x.  I suspect there will be 
> a 1.15.2 before the end of the year (though this might end up being 
> called 1.16.0, see below).
>
>     _I've some pending questions regarding current code contributions:_
>
>     1) ISIS-1636 [1] You wanted me to contribute the java 8 Time
>     Support code, but closed my PR.
>
> I don't think I closed your PR, or at least not intentionally.  That 
> said...
No worries.
>
>     I think, I'm now able to commit to the apache/isis project on
>     github directly. Shall I create a new branch with
>     'dev/2.0.0/java-8-time' on [apache/isis] or on my fork
>     [andi-huber/isis] or what's your plan and/or advice? (What branch
>     should I base this new branch on, if at all?)
>
> ... yes, I think you should create a branch in apache/isis rather than 
> your own fork.  If you want to branch off 
> "dev/2.0.0/ISIS-1276-dn-5-1", that's fine.  Or, you can even just 
> commit directly to that branch; I won't be touching it for a while, I 
> think.
>
> However, it occurs to me that we should double check whether DN 5.1 
> has built-in support for java8 time anyway... now that I think about 
> it I have a sneaking suspicion that it might.  If so, that would 
> actually render your PR surplus to requirements.
Not sure if I understand ... DN 5 has built in java 8 time support - I'm 
using it over half a year now. (The add-on library that provided this 
for DN4 is no longer needed.) However, ISIS does not know what to do 
with these new value types, when it comes to rendering. Hence my PR 
regarding ISIS-1636. (Not to confuse with ISIS-1015 which is active on 
your Jira Board.)

I like your suggestion to branch off on dev/2.0.0/ISIS-1276-dn-5-1.
>
>     2) ISIS-1698 [2] - CollectionLayout.defaultView not honored. Am I
>     alone having this issue? (Could you reproduce it/did you see it on
>     your side?)
>
>     This fix [3] is not yet merged into master. You commented:
>
>>     The pr-84 fix, while perhaps correct, has an impact on any code
>>     written exploiting its behaviour. Maybe that's fine, in which
>>     case will just revert *this* commit. But will look at in more
>>     detail first.
>
>     Anything I should be aware of or look into? (I'm using the fix in
>     production.)
>
>
> I spoke with Johan about this, he confirms it is an issue.  I just got 
> "cold feet" about applying the change because I didn't quite 
> understand what I had broken that required the fix, and I wanted to 
> get 1.15.1 out of the door because I knew it had some other important 
> fixes.
>
> I think that this fix [3] should go into master, with a view to it 
> being in the next release 1.15.x release (or 1.16.0 if we call it 
> that).  If you want to merge it in there, please do.
>
I'll merge this into master.
>
>     3) ISIS-1740 [4] - New Feature: Where am I
>
>     I'm using it in production. Feedback is positive. I'm sorry having
>     to confess, that the existing breadcrumb solution is disabled in
>     all my projects. Do you think there is potential to pull the new
>     feature into core?
>
> I don't think the current breadcrumb solution has many fans, so no 
> apologies needed.  Very happy to try out a better approach if you have 
> one.
>
>     I think the concept is strong. However, my 'proof of concept' [5]
>     code - though work's perfectly fine - would need some refactoring
>     to better integrate with existing framework mechanics. That's
>     something I'm not sure I could do on my own.
>
>
> Perhaps you could create a branch off master and merge it into there?  
> Then I could tidy it up and merge it into master.
>
The implementation uses java 8 code style, so I guess it won't work with 
the current master branch. Anyhow it can wait until there is a joined 
2.0.0-SNAPSHOT branch. For anyone interested to try it out, the source 
is already available here [4].

Regarding Java 8 in general,  I don't know if you heard about the 
vavr.io project. I found 2 classes they introduced highly useful, namely 
... Lazy and Try.

I have my own customized variants of these and I literally would not 
know how to survive without 'Try' [7] in Lambda-Land.

Maybe it's a topic for discussion on its own, how to accumulate the best 
ideas out there into ISIS to be fit for java 8+. (You already discussed 
lombok, which I find indispensable.)

> Since this is a new feature, this is reason enough to call the next 
> release 1.16.0.
>
>     4) I'm having a difficult time understanding how to not pollute
>     the git tree. Having yet learned some basics, I'm still confused
>     and lack some best-practice experience on how to organize branches
>     in a way that is less confusing. If you have any hint for me to
>     cope with that, that would be much appreciated. Otherwise I'm
>     learning by doing, I guess. (I better stay on my fork by now,
>     instead of contributing directly ... I don't know.)
>
>
> Are you unsure about git, or about how we're using it in the isis 
> project?  If the former, I can provide some pointers.  If the latter, 
> we probably will have to work it out (on dev@ ) as we go along.
>
the latter
> When I'm working on Estatio with Jeroen and Johan, we tend to use 
> rebasing as a strategy.  That is, we'll each have our own branch, but 
> before we merge back into master, we'll do a rebase first.
>
> As a slight variation on this, I actually like to do a git merge 
> --no-ff before hand.  To my mind this gives the best of both worlds... 
> the full history of commits is preserved, but the merge commit acts 
> like a "squash".
>
> For example:
>
> dev A: git checkout -b ISIS-1234-cool-thing
> dev B: git checkout -b ISIS-4321-other-thing
>
> dev A and dev B hack away, committng on their branch
>
> dev A finishes, and so does:
> git checkout master
> git pull   # updates master to origin/master; no commits so nothing 
> changes
> git merge --no-ff ISIS-1234-cool-thing   # brings in the branch as a 
> single merge commit
>
>
> dev B finishes, and so does same thing
> git checkout master
> git pull   # updates master to origin/master; dev A committed and so 
> master moves forward
> git checkout ISIS-4321-other-thing      # sping back to dev branch...
> git rebase master                                 # ... and rebase on 
> top of updated master
> git checkout master                              # back to master in 
> order to do the merge commit
> git merge --no-ff ISIS-1234-cool-thing
>
> ^^ the above could be streamlined, this is the long version.
>
>
I need to digest this. Seems that information is exactly what I was 
looking for. Thx again!
>
> Let me know what you think, and we'll move some of this onto dev@
>
> Cheers
> Dan
>
>
>     Thanks in advance.
>
>     Cheers, Andi
>
>     ---
>
>     [1] https://issues.apache.org/jira/browse/ISIS-1636
>
>     [2] https://issues.apache.org/jira/browse/ISIS-1698
>
>     [3]
>     https://github.com/andi-huber/isis/commit/25c3f8e357f090d09d4592b4d68a3afbe850ae75
>
>     [4] https://issues.apache.org/jira/browse/ISIS-1740
>
>     [5] https://github.com/andi-huber/isis-addon-where_am_i
>
>
>
> [6] 
> https://cwiki.apache.org/confluence/display/ISIS/ApplibCandidatesForRemovalInApacheIsis2
[7] http://www.javadoc.io/doc/io.vavr/vavr/0.9.1