| Re: [che-dev] PRs review proposal |
_______________________________________________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
--
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
Ukraine