Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [che-dev] PRs review proposal
Hello Roman,
The proposition and the spirit are good, but i have some remarks:

The devfile you are proposing is replacing the che-theia editor by one that you have built yourself. This is good but I cannot trust it: what if you forgot to push few files or didn't get the right branch ? This is something that a CI could provide for instance our happy path test that would try the PR would at some point produce these artifacts/images. Anyone trusts the CI.

This is also not in the devfile spirit, you should share the original devfile that you are using for developing but change the branch you are using. If it doesn't work, it means that maybe we are missing something and have to improve Che-theia to provide the best review experience. Very soon with Github PR extension, we would even be able to make comments from Che-Theia and amend the PR from someone else, etc ...


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 2: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

Back to the top