Thank you Mario. Some comments inline.
On Tue, Nov 27, 2018 at 6:46 PM Mario <mario.loriedo@xxxxxxxxx>
wrote:
@Sergii Kabashniuk <skabashn@xxxxxxxxxx> for the sake of
"transparency" I am going to provide my (personal) point of view on the
issue you are raising:
*Lack of Unit Tests*
That was already tracked in this issue
<https://github.com/eclipse/che/issues/11980> and I would suggest
that you open an issue as this one if you spot some area in the code that
are not well covered. That will make it easier to prioritize and involve
contributors.
I tend to disagree with the general statement of this paragraph.
Yes. "Clearly" declared POC can have an only source code. Code that we are
putting in the master of Che repositories must contain "UnitTests"
according to its specification as an unseparated piece. The necessity to
have a reasonable maximum of unit testing is "code owner" obligation. It is
something that should not be prioritized or planed separately. Tests are
not a feature. I don't know how to compare a reasonable number of tests
with a new feature. For some reason, we don't have this issue with Java
Unit test. My directly raised
in GitHub comments questions were mainly related to Go-based code.
I don't see a reason (except to get some time to study) why we should have
approximate coverage percent dramatically less than for Java-based
code.
*Use of branches and forked repos*
The repositories/branches you mentions are all temporary. Please
let me know if there are some repositories that we do not plan to merge
because that's something that need to be fixed!
Unfortunately, I'm not an owner of that repositories and I don't
know the amount of work that has to be done to contribute them back to
upstream. However, I can volunteer to identify them, create and track the
progress in one issue and have full my assistance to do that anyone who
needs help.
*Reduce quality standards to do something quick for some event*
Quality is tracked by our QE team. There hasn't been any particular
high rate of regressions in the last period. In the contrary there have
been some great improvements. Of course Che 7 workspaces are not covered by
QE yet but this is still considered a "tech preview" so that's normal. But
before and that's the problem we need to have been doing code reviews for
every, we haven't broke
By quality, I meant Unit test first of all. I think one of the traps
I've got here is that not clearly understanding what level of requirements
to the code should be applied. Is this POC or is this production
ready-to-go code?
*Major code developed in github repositories non belonging to
eclipse org*
Again, this has been used for initial development of "tech preview"
features. I don't see any issue with it. We need to get rid of these
repositories before releasing Che 7 as beta.
I don't see the problem either if we have a clear understanding of
what is it, how to use it, why we using it.
*Tracking upstream Che related issues outside eclipse/che*
I agree with you on that one. We need to be more careful with that.
Asking the issue author to move it upstream or adding a comment on the
issue may be enough for now.
*Usage of https://github.com/ws-skeleton/
<https://github.com/ws-skeleton/> repository*
I am going to repeat myself but...it has been used for initial
development of "tech preview" features and I don't see any issue with it.
And about your proposals:
*Repository with zero number of unit test is not an option*
We should definitely avoid merging code without unit tests in
master. But 1) we cannot enforce such a rule on forks or branches where
preliminary work has just been started 2) code reviewers should be flexible
enough and merge a PR even if it's not perfect (i.e. a new contributors PR,
a urgent fix etc...) 3) quality is not about unit tests only: there can be
awful code covered by awful unit tests and brilliant code with no unit test
and I would rather merge the latter.
I tend to agree with a general statement. The trick for me is to how
to send a clear message about at which stage we are. We are experimenting,
this is POC, we are toooo far away from GA. To make sure that fast POC
nobody understands as quick feature release in GA standards. Sometimes it's
quite hard to fix forgotten technical debt.
*We must move all code to https://github.com/eclipse/che-*
<https://github.com/eclipse/che-*> repositories*
Yes, this is the plan. We need to get that done for Che 7 beta.
*Abandon the practice to hide code development in nonEclipse
repositories*
As long as we track the issues in eclipse/che and cross-reference
them in the PRs done in other repositories I am ok with it. And for initial
development I don't see why we should avoid using non eclipse repositories
if that makes sense for some reasons: we are not sure of what CQs need to
be created yet, we have requested CQs but have not approvals, we want to
prototype something without breaking the rest etc...
*Keep the practice of features-branches*
Contributors should be free to do whatever they want. I am against
any hard rule on that.
Your other suggestions are kind of duplicata of others so I am not
going to comment on them...
My other comments:
1. As far as I can see one of the reasons to use
https://github.com/ws-skeleton/* repositories is our CQ process
that is not fast enough to handle our development speed. I see such action
items here:
- Think about even more fast CQ process.
- Think about something https://github.com/che-incubator/*
- Rules how https://github.com/che-incubator/* goes ->
https://github.com/eclipse/che-*
2. idea development process.
idea -> poc -> beta -> production-ready code in master of
https://github.com/eclipse/che-* repositories
I think one of the reasons for this conversation is that it was not
very clear to me at what stage we are in this process for each piece of
code.
Any thoughts about how to make it fairly simple to avoid
misunderstanding and frustration about an unintended complaint?