[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [che-dev] PRs review proposal
|
Both approaches have value. Often I do not have time to verify an issue
that I have reported. I often do not do a
full Pull Request Review and having one click way to test the PR would
help me tremendously.
OTOH the flow Sun suggested is extremely important and helpful when I am
doing a full PR.
I do not think we need to select one over the other though.
Can we build a bot that adds the devfile, CheEditor and chectl
information to the PRs as part of the CI flow?
Thanks,
Gorkem
On 2 Aug 2019, at 10:44, Sun Tan wrote:
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 <https://www.redhat.com/>
sutan@xxxxxxxxxx
M: +33621024173
@sunsengdavidtan <https://twitter.com/sunsengdavidtan> me
<https://www.linkedin.com/in/sun-seng-david-tan-b05a684/>
<https://www.redhat.com/>
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
<https://www.redhat.com/>
<https://red.ht/sig>
_______________________________________________
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 <https://www.redhat.com/>
Ukraine
M: VPARFONO@xxxxxxxxxx
<https://red.ht/sig>
TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
@RedHat <https://twitter.com/redhat> Red Hat
<https://www.linkedin.com/company/red-hat> Red Hat
<https://www.facebook.com/RedHatInc>
_______________________________________________
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