Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [che-dev] Development quality and transparency

Thank you Mario. Some comments inline.

On Tue, Nov 27, 2018 at 6:46 PM Mario <mario.loriedo@xxxxxxxxx> wrote:
@Sergii Kabashniuk 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 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/ 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-* 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/*

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?


On Thu, Nov 22, 2018 at 3:43 PM Sergii Kabashniuk <skabashn@xxxxxxxxxx> wrote:
Hi
I would like to discuss some practices and issues that I think reduce the overall quality and transparency of development. 

1. The code in some repositories is suffering from a low number of tests that is not cover functionality. 
   Example [1] or [2]
   What I can see it's mostly GO-based. But I think if we look closer we can find more. 
2. We hiding issues(problem) with CQ process or undesired to keep code in branches in forked repositories [1] or [3]
3. Sometimes we ready to reduce our quality standards because we want to do something quick for some event [4]
4. We are developing code(major, core, not optional, not a plugin) outside of https://github.com/eclipse/che-* repositories
   and package it back as configuration [5] [6] [7] [8]
5. Issues related to upstream goes outside [9]. I may understand it if it's https://github.com/eclipse/che-* repositories.
   But this concrete case with [4] [9] make me fill that it's up to external company manager to decide should we have a priority to put the code back to upstream.
6. Usage of https://github.com/ws-skeleton/ repository. From [10]  "Build the docker image and push it on DockerHub using github.com/ws-skeleton"

My proposal
1. Repository with zero number of unit test is not an option. POC it's a good time to at least start write testing, later can be too late.
2. We must move all code to  https://github.com/eclipse/che-* repositories and abandon the practice to hide code development in nonEclipse repositories and referencing it as an image in master.
3. I know it's pain, but I see no other way. Keep the practice of features-branches for the code that is not ready to merge in master because of some other reason.
4. We should increase transparency of code development for the community with a clear process:
idea -> poc -> beta -> production-ready code in master of https://github.com/eclipse/che-* repositories
5. Keep single backlog. Disable all issues pages on all https://github.com/eclipse/che-* repositories except Che
 BTW with new Type A/Type B process it should no be SOOOO painful. If it's not the case then I think we should identify a concrete problem and fix it instead of running away from it.   

Links
[9] Merge Theia plugin broker upstream https://github.com/redhat-developer/rh-che/issues/1003
[10] KubeCon Seattle and Paris JUG Workshop https://github.com/eclipse/che/issues/11972
--

Sergii Kabashniuk

Principal Software Engineer, DevTools 

Red Hat Ukraine

skabashniuk@xxxxxxxxxx    

_______________________________________________
che-dev mailing list
che-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/che-dev



--

Sergii Kabashniuk

Principal Software Engineer, DevTools 

Red Hat Ukraine

skabashniuk@xxxxxxxxxx    


Back to the top