Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [che-dev] PRs review proposal

To me, we should not just do things to make our life easier ... but to make our customers and endusers life easier ... first.
So this kind of flow really makes sense if our end users or customers could also apply it. But they could do that without che, it's like you are working on a Java webapp and provide the war at each PR. is it something we shouldn't do manually but has to be generated by the CI. But then it is not related to Che at all. Anyone could do that with any CI job.

So what is the value of Che for PR and review ?
The ability to recreate the developer environment. for a specific PR: rebuild in the same condition a CI would do but in Che, you could access to it, debug, run your application or your microservice, modify the code and propose changes. With the PR extension, you could add some comments and review. See the diff with another branch with your git tools ... etc.
It really makes sense in our case where we don't have to recompile everything: testing the vscode extension is just about rebuilding the vscode extension and restart it. But it also make sense when you are building microservices, cloud native apps...

I know that this flow is not perfect at the moment because we are missing this and that but this is where we want to go for us and also for our end users and customers because this is where we and they will find value in Che.


Sun Tan

Senior Software Engineer

Eclipse Che - CodeReady Workspaces @ Red Hat
Paris JUG leader

Red Hat Paris

sutan@xxxxxxxxxx    
M: +33621024173    



On Thu, Aug 1, 2019 at 3:55 PM Vitalii Parfonov <vparfono@xxxxxxxxxx> wrote:
I like this approach, because  sometimes really hard to rebuild all images on own side take a lot of time. 
So my big +1 to Roman proposal and I don't follow why Sun against. 
It should make life easer.


On Thu, Aug 1, 2019 at 3:39 PM Serhii Leshchenko <sleshche@xxxxxxxxxx> wrote:
Really, really cool proposal!

Mostly my changes are related to Che Server and lately, I started to provide
 instructions on how to get patched environment and test my changes.
I found it important because a few times I built code of PRs that I was reviewed
and discovered bugs during testing.

So, in my opinion, it's full of sense.
And for Theia it can be simple cheEditor component section that can be pasted in any Devfile,
or Devfile ready-to-use with needed things for testing, like some java project if PR brings some
changes to Java LS.

For Che Server, it can be chectl command and contains needed options and dockerimage with Che.
At the same time, we can try to automate it, and make our CI build Che Server image when it performs
`ci-build` job and push it with a special PR specific tag.

So, my +1 for the idea is here.
Contribution some section to Github PR templates may be a good start point.

On Thu, Aug 1, 2019 at 3:20 PM Roman Nikitenko <rnikiten@xxxxxxxxxx> wrote:

Hello all!

I would like to share some of my experience related to the code review of PRs.


I start to provide for my PRs the section for devfile with component of cheEditor type. The section allows to start a workspace from a devfile and get assembly with my changes in a minute, try to use it/test it.

Sure I understand that this is my responsibility to try giving the best quality for my PRs.


Why I start to do it?

When I do code review for a PR, it’s important for me not just review a code, but try using a new feature (if it’s feature) or test all known me cases (if it’s fix for a bug). So I have to spend time to get assembly with a PR changes (build images and so on).

At the same time we have the cool feature in our product - opportunity to start workspace from a devfile with corresponding component and easy get assembly with changes.


I think we can provide such section for our PRs, so anyone willing could play with changes. It gives ability:

- to experience a new feature not by screenshots or video, but trying to use it 

- identify the potential problems not after merge but before merge

WDYT?

Roman


_______________________________________________
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


--

Serhii Leshchenko

SENIOR SOFTWARE ENGINEER

Red Hat 

_______________________________________________
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


--

VITALII PARFONOV

PRINCIPAL SOFTWARE ENGINEER, CHE PROJECT

Red Hat EMEA

Ukraine

M: VPARFONO@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