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
