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



On Tue, Nov 27, 2018 at 11:16 PM Sergii Kabashniuk <skabashn@xxxxxxxxxx> wrote:


On Tue, Nov 27, 2018 at 10:35 PM Brad Micklea <bmicklea@xxxxxxxxxx> wrote:
On Tue, Nov 27, 2018 at 3:24 PM Sergii Kabashniuk <skabashn@xxxxxxxxxx> wrote:
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?

Would issue tags work for this? A side benefit would be clarity for the person writing release notes about whether a feature was intended as POC, beta or production ready code. That's not always clear for the RN writer either...
For me as a community member if I want to be a part of the process of 
 - problem identification
- part of the decision making
- part of implementation POC
- early reviewer and adopter.
- to use ws-skeleton/* or che-incubator/*  as a playground.

What those repos(?) intended for? What is lifecycle of things inside? Why I (as a community member) should/can/must use it?

Would issue tags be enough? I guess no. At least not for all tasks.
_______________________________________________
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    

_______________________________________________
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

Back to the top