Effective Code Review

Video thumbnail (Frame 0) Video thumbnail (Frame 1029) Video thumbnail (Frame 1938) Video thumbnail (Frame 3058) Video thumbnail (Frame 4105) Video thumbnail (Frame 5094) Video thumbnail (Frame 5961) Video thumbnail (Frame 7191) Video thumbnail (Frame 8396) Video thumbnail (Frame 9656) Video thumbnail (Frame 11317) Video thumbnail (Frame 12525) Video thumbnail (Frame 13434) Video thumbnail (Frame 14328) Video thumbnail (Frame 15346) Video thumbnail (Frame 16227) Video thumbnail (Frame 17766) Video thumbnail (Frame 18643) Video thumbnail (Frame 19767) Video thumbnail (Frame 20634) Video thumbnail (Frame 21698) Video thumbnail (Frame 23036) Video thumbnail (Frame 24975) Video thumbnail (Frame 26135) Video thumbnail (Frame 27197) Video thumbnail (Frame 28605) Video thumbnail (Frame 29599) Video thumbnail (Frame 30601) Video thumbnail (Frame 31469) Video thumbnail (Frame 32678) Video thumbnail (Frame 33682) Video thumbnail (Frame 35145) Video thumbnail (Frame 36084) Video thumbnail (Frame 36977) Video thumbnail (Frame 38041) Video thumbnail (Frame 38889) Video thumbnail (Frame 39778) Video thumbnail (Frame 40680) Video thumbnail (Frame 41733) Video thumbnail (Frame 42867) Video thumbnail (Frame 44187) Video thumbnail (Frame 45047) Video thumbnail (Frame 45991) Video thumbnail (Frame 46968) Video thumbnail (Frame 48432) Video thumbnail (Frame 49340) Video thumbnail (Frame 50415) Video thumbnail (Frame 51989) Video thumbnail (Frame 53336) Video thumbnail (Frame 54691) Video thumbnail (Frame 55697) Video thumbnail (Frame 57738) Video thumbnail (Frame 59166) Video thumbnail (Frame 60028) Video thumbnail (Frame 62048) Video thumbnail (Frame 63034) Video thumbnail (Frame 64589) Video thumbnail (Frame 66233)
Video in TIB AV-Portal: Effective Code Review

Formal Metadata

Effective Code Review
Title of Series
Part Number
Number of Parts
CC Attribution - NonCommercial - ShareAlike 3.0 Unported:
You are free to use, adapt and copy, distribute and transmit the work or content in adapted or unchanged form for any legal and non-commercial purpose as long as the work is attributed to the author in the manner specified by the author or licensor and the work or content is shared also in adapted form only under the conditions of this license.
Release Date

Content Metadata

Subject Area
Dougal Matthews - Effective Code Review Developers usually state that finding defects is the primary motivation for doing code reviews. However, research has shown that the main benefits of code reviews are; knowledge transfer, team awareness and finding alternative solutions. Code reviews when done well are more than just finding defects; it should be a discussion and conversation with other developers about finding the best solutions. We will talk about re-framing code review to encourage open discussions. ----- This talk is for everyone that is already involved in regular code review and those hoping to start. I will talk through the code review process with the aim of making it a better and more useful experience for both the authors and the reviewers. The talk will follow the following rough outline: - Introduction - Why do code reviews - What are we aiming to get out of it - Submitting code for review - How can you help reviewers? - What should you avoid doing? - Removing ownership of the code - Reviewing code - How should you give feedback? - What should you look for? - How can you encourage people to review more? - How to avoid and remove bike-shedding - Code review tools and how they impact on the process. - Wrap up and conclusion
Wiki Context awareness Roundness (object) Red Hat Computer animation Lecture/Conference View (database) Multiplication sign Machine code Machine code
Integrated development environment Multiplication sign Quicksort
Computer animation Link (knot theory) Dualism Machine code Chord (peer-to-peer) Twitter
Mathematics Lecture/Conference Semiconductor memory Personal digital assistant Projective plane Videoconferencing Machine code Computer programming Number
Red Hat Open source State of matter Autocovariance Software developer Multiplication sign Projective plane Online help Water vapor Mereology Machine code Twitter Process (computing) Lecture/Conference Form (programming)
Functional (mathematics) INTEGRAL Disintegration Bit rate Complete metric space Average Machine code Number Optical disc drive Bit rate Average Radius Software testing Contrast (vision) Execution unit Red Hat Projective plane Sampling (statistics) Machine code Unit testing Computer animation Function (mathematics) Contrast (vision) Software testing Quicksort Spectrum (functional analysis)
Word Process (computing) Computer animation Software developer Meeting/Interview Multiplication sign Software developer Machine code Average Machine code Vector potential Number
Expected value Process (computing) Red Hat Computer animation Link (knot theory) Lecture/Conference Shared memory Sound effect Mereology Machine code Computer programming Number
Addition Curve Context awareness Software developer Heat transfer Heat transfer Machine code Expected value Exterior algebra Computer animation Lecture/Conference Internet service provider Quicksort Resultant
Context awareness Graph (mathematics) Red Hat Multiplication sign Optimization problem Sound effect Distance Machine code Number Data management Computer animation Term (mathematics) Computer configuration Order (biology) Software testing Heat transfer Computer-assisted translation Family
Point (geometry) Functional (mathematics) Computer animation Lecture/Conference Connectivity (graph theory) Telecommunication Multiplication sign Projective plane Software testing Heat transfer Machine code Machine code
Data mining Computer animation Lecture/Conference View (database) Mereology Local ring
Point (geometry) Collaborationism Computer animation View (database) Core dump Collaborationism Disk read-and-write head Machine code
Revision control Collaborationism Dependent and independent variables Mathematics Process (computing) Computer animation Lecture/Conference Multiplication sign 1 (number) Bit Figurate number Quicksort
Process (computing) Computer animation Open source Projective plane
Web page Red Hat Electronic program guide Projective plane Machine code Lattice (order) Word Process (computing) Computer animation Vector space Integrated development environment Computing platform Software testing
Context awareness Context awareness Lecture/Conference Meeting/Interview Software developer Projective plane 1 (number) Software testing Machine code Computing platform
Point (geometry) Slide rule Context awareness Statistics Focus (optics) Red Hat Length Projective plane Content (media) Electronic mailing list Student's t-test Machine code Medical imaging Mathematics Computer animation Lecture/Conference
Software developer Feedback Line (geometry) Machine code Machine code Order of magnitude Formal language Mathematics Roundness (object) Process (computing) Computer animation Lecture/Conference Object (grammar)
Linear regression Divisor Patch (Unix) Multiplication sign Set (mathematics) Disk read-and-write head Mereology Likelihood function Machine code Mathematics Coefficient Lecture/Conference Mapping Patch (Unix) Computer file Thermal expansion Power (physics) Number Computer animation Grand Unified Theory Logic System programming File viewer Quicksort Family
Linear regression Patch (Unix) Computer file Mereology Power (physics) Machine code Twitter Number Word Coefficient Computer animation Factory (trading post) System programming Data structure Data conversion
Point (geometry) Context awareness Dependent and independent variables Focus (optics) State of matter Feedback Multiplication sign Patch (Unix) Feedback System call Degree (graph theory) Mathematics Sign (mathematics) Computer animation Integrated development environment Personal digital assistant Authorization Text editor Arithmetic progression Social class
Point (geometry) Computer animation Lecture/Conference Feedback Machine code Neuroinformatik
Dependent and independent variables Computer animation Lecture/Conference Projective plane Shared memory Authorization Mereology
Mathematics Word Dependent and independent variables Meeting/Interview Projective plane Authorization Machine code Resultant
Category of being Mathematics Red Hat Meeting/Interview Lecture/Conference Forcing (mathematics) Projective plane Quicksort Lattice (order) Computer programming
Dependent and independent variables Multiplication sign Projective plane Set (mathematics) Line (geometry) Machine code Formal language Order (biology) Mathematics Word Process (computing) Computer animation Meeting/Interview Personal digital assistant Summierbarkeit
Web page Rule of inference Group action Red Hat Divisor Open source Patch (Unix) Projective plane Frustration Mathematics Process (computing) Computer animation Lecture/Conference Social class
Focus (optics) Open source Multiplication sign Projective plane Shared memory Machine code Checklist Variance Number Mathematics Programmierstil Exterior algebra Lecture/Conference Meeting/Interview Software testing
Point (geometry) Computer animation Information Electronic program guide Extension (kinesiology) Disk read-and-write head
Slide rule Multiplication Mathematics Computer animation Lecture/Conference View (database) Mehrplatzsystem Source code Maxima and minima
Game controller Arm Software Meeting/Interview Lecture/Conference Machine code Line (geometry) Distance
Computer animation Lecture/Conference Moment (mathematics) Point (geometry) Feedback
Computer animation Lecture/Conference Moment (mathematics) Point (geometry) Machine code Traffic reporting
Point (geometry) Computer animation Negative number Line (geometry)
Dynamical system Data management Integrated development environment Open source Lecture/Conference Meeting/Interview Personal digital assistant Speech synthesis Distance
Computer animation Query language Collaborationism Authorization Machine code Data conversion Thermal conductivity Wave packet
Open source Multiplication sign Feedback Projective plane Collaborationism Shared memory Product (business) Word Mathematics Computer animation Lecture/Conference Form (programming) Social class
Point (geometry) Touchscreen Computer animation
Computer animation Lecture/Conference Universe (mathematics) Feedback Water vapor Traffic reporting
Constraint (mathematics) Computer animation Source code Summierbarkeit
Type theory Computer animation Open source Lecture/Conference Logic Feedback Basis <Mathematik> Rule of inference
Point (geometry) Word Computer animation Information Feedback Projective plane Machine code Spectrum (functional analysis) Number
Type theory Red Hat Computer animation Lecture/Conference Coma Berenices Spacetime Stack (abstract data type) Quicksort Open set Twitter Spacetime
Collaborationism Projective plane Machine code
Statistical hypothesis testing Type theory Message passing Commitment scheme Computer file Term (mathematics) Projective plane
Email Red Hat Electronic mailing list Mathematical analysis Bit Instance (computer science) Power (physics) Wave packet Latent heat Data management Arithmetic mean Process (computing) Self-organization Arithmetic progression Information security
Point (geometry) Touchscreen Multiplication sign Projective plane Computer programming Type theory Radical (chemistry) Mathematics Process (computing) Meeting/Interview Universe (mathematics) Metropolitan area network Resultant
Axiom of choice Point (geometry) Implementation Dependent and independent variables Red Hat Software developer Feedback Energy level Machine code Lattice (order) Arithmetic progression
Implementation Mathematics Red Hat State of matter Average Software developer Right angle Branch (computer science) Machine code Line (geometry) Hand fan
Point (geometry) Implementation Red Hat Multiplication sign Projective plane Feedback Bit Machine code Mathematics Latent heat Process (computing) Repository (publishing) Analytic continuation
Expected value Mathematics Group action Meeting/Interview Order (biology) Projective plane Machine code Line (geometry) Analytic continuation Software maintenance
so our next speaker give a big round of applause from Google Matthews and at the global view they're the and so I've been
that will 1st of all thanks coming and for having me here having thinking about what could
be called quite some time now someplace sectors of talk about it and they have just start discussion about it after my my main goal from today is that people are talking about code you more than that success and so the 1st of all just give you some context
of where I come from and why do use naturally to important for this talk page you want to know where I'm approaching produced from I work forever and then I'm working on the
of respect for that which is the the bottom of and and then I walked into blow inside of this that and the key thing about all of these is everything I do is open source and public sold for the that also public and all the time so this means
that some of the things I'm gonna say are specific to a themselves together and we got quite a closed environment by being still things you of want and I also run the 5 lost the use of Scotland and which is completely unrelated but I just want to sort of whenever I can as an inside joke which I can explain literally to understand and I'm also there have been experiment in this
talk i of tweets and and that's why to recount the following dualism 0 will be tweeting out some relevant links to the and if there isn't too out by now there
working and so you will see words that if you're looking for references to any of the of the papers or called I reference literature on what to do 1st of all I decided to
engage the audience will that around and see what people are the code reweighted during an and together baseline how many of you will raise your
hand if I ask you to review and I guess but yeah that's a guest of sense I think we have adjusted to that from now who's doing regular chord
at work and by that I mean that have the overall the review of a change the things that a good percentage of
about 75 that memory and who is doing the now the otherwise so that the open source or in what a side project from 1 again it's all for review that it was a lot less than moving out of the wants and finally to actually does
program you that looks forward to in
that case so we pretty much everyone doing some kind of code review and then the number of people actually so this is something of a video again something else and people actually looking for doing produce fully down to about 20 per cent and this is the 1 that is the most interesting to me and I
find prodigy incredibly valuable so much you get out of it is really useful process the yellow people don't really enjoy it so I want to start a discussion see how we can prove that we can make a better experience verbal I also don't straw poll on Twitter not too long ago and found out that most people are doing covariance sorry elderly people wanted everyone to encode up to 25 % relative to the center of the state of acting as part the development time but I mean that's a huge chunk of time and so that we can prove you can prove what we build or perhaps more importantly that improve help people enjoy their job and for any not that this
talk is really more and more what people are just could lead to you a couple quotes and things which you need to persuade someone white should be tinkered review and all of these forms of some references to that because of what's possible to revert exploits and that as far as I'm concerned the only real reason to not be encoded using your soul developer on what and and you like to find it also this trick that is before you do an open source projects and rather than committing directly to master the European article requesting might find occasionally some people wonder why do you where you where you got especially the the waters a new
project the unification when you look for Western Europe was and so 1st of the support from all of
this sort of that people could complete plus the before and he's he says that the average detection rate for and unit test 25 % that sample function that's what about the simple integration tests and in contrast to the average spectrum of design and code inspections is 55 and so and this is
something which about the defects is kind of odds of problems with the code and and efficient claiming that produce the most effective of all these approaches to improve support and I'm not sure I completely by these numbers Quality of testing that's quite an impressive of a like amount and the next provide Jeff I would want
to he wrote this enabled us and as we said the only had a lot to do with your finding a developer the cross to actually do and what you started off with says I think you couldn't find that every minute spent according to you in the back temples and this this 1 little tackles the assumption people say they just don't have time to decode the
and that really I think there will be some upfront investment cost that wanted to study in the process of it you will find that you will with the benefits of time and finally from a paper of measuring
defect potentials and defect removal efficiency academic names of words and they found it full design and code inspection of the topic but the scientific removal and average around because they're so again this is what the huge number and this is quite this is from an actual an academic paper and they
share some of the data used for that you while method behind these numbers the other 2 were more anecdotal as part of the talk with outside hopefully year-old
convinced now to decode you'll have most European anyway we can move on and that if anyone is actually spelled out in the process for some reason I can bombard you with references and links as it is pronounced afterward and wanting to use some of the things
about produce quite interesting is trying to compare the expectation people have with the outcome of that program and I think this is it reinforced was by sports you'll notice to talking about finding defects involved and I think that the reason they're talking about that is much easier to measure but another thing that you the socalled side effects and outcome for rather than just thinking about the bottom in and in the
paper expectations of outcomes and challenges of modern produce they said that while finding defects remained the main motivation make too much the main motivation for reviews and reviewers of less about defects expected and provide additional benefits such as knowledge transfer increasing awareness and creation of alternative which is the
problem of what that she done and this is this is research was done in partnership with lots of research and my into models with from the and the
they different things that Microsoft because the developers beforehand and sort of question around the quiz and so there often wide encourage you they're expected to get out of it and the the top result was finding defects in because that was the most and that that's why we have been covered by actually when you look at the about sorry and then after the curve user has had had happened in the uh the research team then manually classified hundreds of reviews to many we find out what they call was the outcome of the review and this is the result of the
cat that family found defects was only 14 per cent of the time so that only came in both in terms of the actual from reviews and so it's actually a lot more lower down and you would have expected time I call up to the numbers because the paper had a graph and they didn't actually quite hard to see the numbers that explicitly mentioned those 2 numbers multiplied by only happen the order growth and so you can see
about the effect you're getting good improvements to the distance between the effects could improvement is the submitted for review was perfectly valid and I've worked very well in the optimal solution depending on your context but it wasn't performing enough or maybe there was a that something more what elegantly manager and the different options and understanding with a shared
understanding team about what is going on with the code so but if you're not actually reviewing 10 of coming and the only way you find out what's going and going on sorry it by looking at European history with that of
component or you just defined you something and you're like this function change the point and they're just about making sure that the shared understanding and everyone really knows how the project is developing evolving over time and then they'll be found social communications which is obviously a very important role in the i was also
rated higher than the so was the of
gold mines about it was thinking about rather thinking about them looking for a book on encourage you think about how we can maximize all these other
benefits as well I mean obviously finding defects is still important part of the solution will be focused on that this is going to be made at a of the inquiry we definitely have 2 distinct roles you have the and reviewer or you can have multiple values that side of the review because and and then I go back to while locals and have a negative view could you I think something because the the actual
core deposits comes along is almost an adversarial like you working against each other rather
than together and let you you want your private that this person is giving way by giving you like be like and this is kind of tricky problem to solve it's a moral issue what happened I think this is definitely more prevalent in some
situations and others and people more extreme your point of view of the fact that it works with the by the 1 the reason that we see this as a I don't like the the the
name for at all like a username and I don't really think we will manage to change in this budget the sales and buy at least in my head I like to think of it more as we could discuss in corporate collaboration displayed the
thinking more about how we can work together as a team and because of the process your but involved in yes differently about collaboration coming with that switching to get the that essentially have a single long time and although I can sometimes seem a bit of a conflict of the time when you can't figure out what the best solution is to
and so all of the nouns is take a look at all the changes in the market to looking reviewing the laughter and it's sort of a few bits of the ones that survive response from either side and and you can you
can begin to use which is useful this first one I think is really really
important and perhaps overlooked because you might think that comes before the review process with you don't really and important especially in open source projects to talk about what you want to do 1st and if you know that you don't have an issue or something describing what you told by not discussed it wouldn't in need only know that the football or relevance to the project and obviously this is something you can judge yourself and that it is likely that the no-brainer you should just send over
progress and or sometimes you might find you can express what you want to describe vector of the set to be prepared that Europe for the story of might be rejected just because you haven't actually make sure everyone on the same page and I think it's also important in the sense of having been work environment you expect to have like requirements for the the something before you start in the code that is making sure you know what you're doing before for you for your and it's always good to adhere
to the guidelines is is pretty obvious that a lot people died you tend to look for them and this is also perhaps a problem with the project followed and you don't have more than make making too hard to find and we can always take a look at see how the project is working in general other review which are waiting or commits the words just trying to fit in with how they're working in a meeting the process a lot that they got
did the role was like adding tests and documentation of everything in the testing of platforms and it's always good to make you're doing the same thing with the development of providing
context is really important when you come or with your during sorry review I I find the reviewing code can be really quite difficult often you'll and you get you receive a thing for a project the unity of all of the
procurement so you need to like bring back the mental context of what it what it was doing before and then figure out from the data what they're trying to do now I just want to get out of here if you want to be to get around this by writing good than communities to pull
refused to y you making change what they want what does it to this point is descriptive stats and up and up and you missing the context of this image by the way some of his students not to that I don't know have and this is a
list of related to the previous slide way keeping you changes small and contain this is about limiting the content required so you don't want somebody to you have to understand the full breadth of project this will something you want to be doing uh a small change in keeping your change focus what the actual topic is you some people like to to go and do to 1 thinks farther north spend the notes and you over a
thousand languages they will run within the rounds of thinking that kind of money and that money be perfectly acceptable they spent objectives and these 2 different uh review review requests and and the
history as seen a while ago and I know when I was very often it is not the down as this is saying CodeView 10 lines of code magnitude 5 everyone's heard of fine and for this is basically I
mean the some developers might look at this and think of sales and technical process of what these would be much more and that's that when although I would think about it and I think the reason you get more feedback from a small change especially if is much
easier for the viewer get that change to their head and it's really in the brain wrapped around and where that you're trying to understand much larger change you end up missing things because you focus so much on the overall impact of it and that was simple subtleties wilderness of and that this is
always being kind of a gut instinct for me that you're sort small of patches were easier or more efficient to but to review and that is actually quite interesting papers this 1 is the investigating could be equal to the people and parts the feature map of the family and they found that logic patches lead to a higher likelihood of reviews missing some of so rated a set of the family like expansion of its own and this criticism factor with data because they may spend some time looking into the public the
words of tractor and they the narrative together with the also note the probable Mosley's but what about structure and the reviews and the combined intuitive and so find the trend of the bigger and factories were really the part of the solar disappeared much less false
when you have to submit you review I think it's good to think about that as being the start of the conversation you find on some people will also be review and also the water and that the favor the available send over and they say taking you adjust weights and
the model and on the legislature found composition is much better to send over the current state and you have a look at this and tell me what you think you basically inviting feedback and making that the Bible is much clearer and I've had so that had plenty of time for class us and the people of this you know on the back of the for itself with this eventually you can find schools and things of don't progress and 1 100 trick can find when you so when you're doing this call request actually or do you think will but I just think of it in general and not to the degree of the of and the uh when you're when you open this request for a review and I was the like going out to do a quick review myself so I'll go and stand on the of on character and I find that I often find mistakes in my own patch at that point I'm not sure why I think maybe it's the the cheap context from an editor to recover the dual of this environment and so just out of school of which I recommended and so
when you actually have your changes up you will need to try and link with furniture so that history was the response to the so they put your whole life around about how much time you spend in cognitive and to me that's just a sign of a really politics culture and I'm not sure if it's the focus of the reviewers all the authors this changeover in in this case sorry all understand what tell without more context but it's just as the the author of the changes will be
important to just can't think analytically and objectively evaluating you could and at this point because people sometimes you feel quite particularly you quite proud of it you you become tied up with that they might code works and then when somebody then start to give feedback on the computer quite a lot of sense and the the way
I like to think about is if you if you got 6 months ago he probably find to describe things about you right today like you lots of money and
so really I I think reviewing code is pretty difficult and so on the whole thing is not trying to make it easier for the reviewers to review vote and give that to try and make the whole process so that help them to help you
and and then when we look at it from the review part you and that the kind of the gatekeepers for the for the Timor-Leste before coming in is that they have a responsibility to
make sure that the you're coming in is good and suitable for the project and this is this is just like the authors have the same responsibility to finding the best of and the key thing really is if you want to make sure you're sharing responsibility and I I can
see the reviewers of sorry the reviewer of code over the years
and has been responsible for the change as the author and I think this is where some tools like the flavor of the end of the project result quite often just the author of the change and depending on how the occurrence of the word submitted eventually and the interesting and we can have a better way of integrating because children you give way more likely be the people are behind this change of these 2 people and that the that it is better to share responsibility this 1 is the only
force on that happens often so the so that is in reference to apologize don't don't know where it came from originally the code that that says something like program sort and in my work the court says something like
contributions of like properties everyone like properties but you need to look after the war many people similarly with currently you given that you need to make sure that documented in the literature that they need to make sure it works for forever and so as a reviewer or project you should really feel like you can quite happily say rejected change just the and
sorry I I can't maintain a set of language in the and those a case what I'm working on so I wonder what it's going in this and get out and then I have a pool of blood so that and the change came in it with reasonable it wasn't very big and the story I don't think this is important the time and their their responses from the lines of sum of all must need to do is just magic and actually you asking me to make sure this is something that works and so were able to move from the project and that can be quite difficult and so what you never know what people are that's that's
really important that everyone reviews and I think it can be sometimes people would assume that the better for just the seniors to review project and then 2 years of work the code that feel like they come to you and help responsible for the review and and this is actually technically through the use of the word and I have lost a reference I have about people popular confined online and they basically they found while city reviewers were doing a better job of finding defects and given the that the best way for continues to actually level off and become more useful and become seniors essentially what to do for and so you
need everyone to do it and you also remember there's a whole lot of benefits coming onto this show understanding of what's going on you you know about this than those in the stages of and and the other thing about here rather split in between who reviewed the doesn't you can find that can create a divide between the 2 the 2 sides and this is something more about competition against each other the just make sure everyone is the process and so you can join in more with an open source project and group you can review of Europe and also smitten changes all and maintainable absolutely love you need to review the few other open for class of patches and 1 of so that you
don't want everyone reviewing is really important to keep them all on the same page that can be a very frustrating process for contributor you've submitted your 1 review and then the objective another review from someone else in the picture back to where you were originally amateur and they just really trying to achieve a shared understanding of what is expected from your country you can do you
can make this and the review guidelines you can also have things like review checklist for you review using and following as a guide and so the the capitalist you have to make sure you avoid anything that can be automated there are a number of things that you can automate encoded and by
that I mean making sure that your code style passes whatever linking you want to use and then you go from each of these tests are run and it's just that if you as a reviewer having that's manually you you with the time and it's fairly easy to set these
things up with something like this which is the focus of projects or there's tons of open-source tools which he could on and 1 of the best things about making this is you run quite like salt that against the code and you get negative feedback there it's much better that comes from a change in the law from of of like an automated rather than from a human you find that you have your review is actually manually commenting on code style and they're just wasting their time and it's far more and he and his project and so that the nice thing about using automated tools you got you have a share and hatred of that told can also at that really the alternations about removing the
point so for these these kind of discussions people like that they could style is not tested automatically then ignore the comment as far as I'm concerned that if you want to use the style guides about something then new like a like
a extension of what information mother and if you're not familiar with 1 but by telling what explain right now but just get invited talk on this basically people will debate the simplest details rather than focusing on the actual real problem and and I think some technical over the head and yet summary and there's there's not much to say
about this and I have a slide for it just because I think some people will assume maybe 1 reviewer per change the really I think you should consider always try multiple reviewers in OpenStack we we have many
review the minimum of 2 and so that's what I find really useful and I think it be multiplied the benefit from some of the subsite difficulty getting caught view of understanding what's going on and and 1 of the things that's nice about multiple users you actually to source of what when you're reviewing
it's important to major European crush and and that you're doing so so you really want to be free control there is a paper which is the impact of code review coverage and code review participation
on software quality and and they found that that produce people arms 201 problem and a bunch of other papers seemed to reach about 100 lines of code in our this distance and so since the very and that it will lower than I expected but the important thing is just to make sure you're you're fine in this not so driving 1 when you're fundamentally fried because this will be doing in thing personally I find my best according use is just after I
started thinking mind strong morning coffee and and the right go I had knowledge and you know that some of our destructive and I'll spend maybe an hour or so to review them when you're
giving your feedback and then review make sure that you are being constructed and you're giving and so are you giving constructive criticism and you also comprised of people who is very easy to get tied up with giving the bad the bad people as saying putting all the problems that really nice if you can point out some of the things that
you learned in the course and it is not think it really helped me it was a small cost and interestingly I was when I was practicing people if you want to go 1 of my coworkers right and sorry I got distracted by e-mail notifications and I want my co-workers actually said something on my report a review it's material like the islands and today the right and know you could have gone a off just about the cost of and I just got a medical coding about it so that I was more encouraged to try and help so share more knowledge in the and and this
kind of falls on a similar line which you been quite unaware of course when you're talking produced and this can be tricky it's very easy if roster 1 this what things that so in this example was there that I was saying this to something that's that's what you want and you do that that sounds reasonable because you in Matoran found from the Sun Valley relaxed name is written down at some point will like why did you do that so I have another
regular voice that year has been initiated and so you can alternatively maybe say something like this instead you know it's just going to be
even larger about things and they'll only make and some of the negative feedback easier to receive and it's really important to me you're asking questions telling so what's the dealers examples you say something like you should do this and you tell them they might have a good reason for the distance to the of and this 1 over the other speech never be harsher never possible in your um companies and I think this is probably less of a problem in the corporate environment or what environment assistance to be a good
social dynamic between people on the streets of abandoned the people and there's always let managers and so on to solve our problems that that 1 more problem in open source communities the and packaging
authority has the code of conduct for um for basic reported in general that covers queries and answered currently that I found out about it because I was assumed that could that the fight for the Union and fight ID plants that allows for the final point of and so yes there is
kind of like writing corridors are reviewing is also hard you want to really try and help out the real with training so again this is something that you and that kind of brings together the the 2 different sides by having this conversation is is about helping each other to problems that was all working together and alternate what can to save time and reduce that kind of burden of the 20 tossed yourself and the country itself by
time was a new word you make you restricting how long he spends doing semantic from earlier by just remember this thing up and 1 of the real
benefits I find with doing reviews is you can actually you can learn a lot about what it took quite often when I'm starting to work on a new open-source project and I want to make some contributions for some reason I'll actually start doing review for I ever submit something all these really careful about my feedback of I
don't really understand what's going on the class and make useful changes and you start to see the benefits of that shared understanding and share learning in the 1st and so is a great way to from the land of the product delivered project is to look at review of what's coming in so you know what the actual form review of saliency see what they're expecting from other people and also it's incredibly useful and you can start you know when
I told people about this for a lot of room so last May was likened to Paris in the course the tools and what what was the best thing to do let's find that's not very interesting and if you wanna look at screens also written documentation of these points and I and really I'm only qualified to talk about the other guy might use them extensively by and
all the others and sure all much more I don't know about that but never really used and the 1 thing have really expect from your you produced make sure you reviewing for merger so rather than say connecting directly
to master the of water in reports and make sure that you are having some review before that
happens so that could be be enough for personal gain of in the summer Garrick overview for for example the reason for this is the feedback loop is just to delayed if you try and review after the universe and the the effort to actually take this feedback to make updates it is just much harder you find the people to review and the sale we should all this but then eventually gets forgotten about unknown does it so you need to keep motivated and only let the code and after enough to these with the fact that
she could we talk about the sum of the 2 and get of every most people here I imagine provenance of the available about it that it's got very loose and undefined workflow and the label that can of useful for like
tragedy and things that is actually quite hard to do any kind of a detailed or complete work for the use of a source about simply why would prefer not that something would be very hard to for example if you want to consider the multiple reviewers who have have some constraints convention to make sure to people and with the and the adjustment of the work that way in the everyone has the of which is a great benefit and it's also very easy to use and quite feeling and they're on
the other hand is pretty much the opposite of that so this open source it's kind of is hard to use it but you can very defined so what was rules in it and you can do a much more things we can have more room here for example and when people do reviewing and there no and
explicitly given the you'll write your comments and then you'll like minus 2 minus 1 plus 1 was to and what they actually mean the really obvious that the native peoples and that the the nice about you you give some feedback which is like our sport this type of Italy matter of still like if you have positive review that you happen to be a basis for some other reason that you could stick that account for a logic to give like this the small feedback which you have it not be like real thinking person and what the review just because of that and 1 of the the really interesting things about having all of this and the data
to there is there's a huge amount of information out there on 1 and this is 1 of the things I would have wanted to them at some point so this is kind of fun looking for like
minded people the end end of the spectrum sampled around 3 100 and that the solvent reviews in the Karo thinker remember the numbers going up final so and you can pull down all the you want 20 K Jason of something but I'm sure there's something we can learn in there and you can think of feedback and there's a whole bunch of other projects using of care and the and the like and we use the word that you know we can to
make you think of you and so there's something to be learned there and or you can have fun with new data so can go and the I think you just quite interesting thing to to do and the and that basically wraps up my thoughts
about have to take questions my contact details and and sort of related to what you do not this type of space more so that it was interested more above the but instead of course you work for the people of the of the of the of the of the
wanting to know if you're looking at that
the the whole interval is 0 I should have become 1 of 1 little on the top I wanna know all this beach who is working on a project alone what's a good way to find people to review my code yeah that that's something
that is a very difficult problem is kind of the eternal problem of say an open-source project is how do you get more people to with your code of you will take you to code and I I have wondered if there was some way we could have like a collaborative like there is a way of networking people are working so those individuals and we can evolve along with agreement to try and review each other's code that could be quite interesting and it also get more people involved and the I don't really have a good solution the right thing to do from heights and I to ask you to
think that reviewing kind of movement issues and there's like your original rejection regions in terms of because because the commit messages and that's clear enough for to the commitments quest is kind of being too picky or those are some of the immediately to basically established clear guidelines in your project and so
in most Wi-Fi projects I wouldn't be too picky about and this is the end of this type people actually and that's 1 of the nice things about guarantees your commit message is actually a file that comes up for review you review people review commit messages evolve and make sure that they are useful and I think there was some value in it but then depends how good everyone with their of keeping history as high
as an actual stockpiles and organ OpenStack so I was wondering if it's just the whole mess in the review comments for instance this large had said that in progress for review and there's an interesting discussion deep analysis there's 1 interesting comment perhaps 25 long paragraph and then it's just boring in there on a mailing list based on past workflow you can link to a specific last comment power in guarantees just boring and sitting in there and like 6 months later refered to it and again you have to link to the review and then there's a whole bunch of 25 CI and jobs listing the status so it's very messy and so you know how to handle that have having this is a specific
problem with care so for people that don't know when you use the military to security someone and delivery and then use the mean of the training the comments layout of the the 1st revision of hidden and so sometimes you might want to do another review before you see the comments you need to actually go and look for the most of the knowledge that you of the and the I think it's just a problem with the carrot you I used to expose this better manager found out recently you can search for all your comments on draft which were never submitted and I've got a whole bunch of review comments which I thought I'd suggested that are just sitting there I found this last week so that's a bit of a nightmare of like that's what it was perhaps thanks and it's more of a winery
yes and there have been any other questions so I was just wondering what your thoughts are on a man called the universe is pair programming yes so this is 1
interesting 1 I think you so what I work remotely for that which means this pair programming is slightly different things and then that we sometimes do things sharing of screen terminal under the of what chance time and I can work quite well that that means I have not done a whole lot of programming that I think they you can basically the same result of the process and the beginning so in OpenStack we still need to have all the reviewers just for the simple fact that the change would be merged until you have enough reviewers and so you care program doesn't necessarily help you out of this type I can see that in some situations you that you could just replace provide you with the community if there's only 2 people working on a project then there's not much point in a formal review the programming I think that resulted in the interesting to note of an experience but compared to it from the point of thanks
just a short response to your question because well work we have assignee for code review a week so before we start working on a new feature for example we come together and discuss the implementation so you avoid having the bag after 3 days were saying that this you know and I don't agree with the implementation choices so that we have found it to be really useful to you know work on a future together in a way that you planted together and discussed before you actually have a symmetrical quest in a lot of what
follows from the beginning of the movie in an artifact of the fact that I am remote work the white while undesirable in my code of review very early on and then Gary market work in progress that allow people to have a little rough look at it from a high level but not a proper review and then you can get some feedback about their and which is another point that is of a very avoid wasting the 5 days of 3 days development of the idea score meetings because it's fun to bring up the question also thanks for
the as DG may be tried to implement strong base development and you know it did you have experience with that encodes you know so this probably
the development is just when everything that's great and yeah everything's goes to the mastery of some of the similar changes all get that essentially lot that everything goes directly into the master branch unless you're like off to previous fully on and the I guess we do that it seems to work pretty well and I have never really been a fan of the long-running branches I find that they called problems which because of the that I'm not I'm not I'm not I'm not sure by some like something else about OK and what size are your average commits like in lines of code and that's a good question and that I look at the state fair enough is that it's going on right I guess I'd like them to be a couple hundred 1 the most and occasionally if you're moving things around and that would really be used and that can be the review of the eye of right up there with you that the thanks that thank you
phone that I wonder if you have someone experiences or ideas of comments about reviewing different things and cold like growing up and we have blueprints and specifications and sometimes called Steiner changes and and so on and so on yeah I think he can you give me that
really careful and this is a problem I think it's coming up like so when you want to some projects we want to make a change you typically repository would give them which perfectly files and you want to submit with the documents where you want to do you that in the feedback process and then take a really long time people really start to go into what you much detail on this back and forth on the Americans do not want to love summaries of having documentation aggressive in a bit more like code and I think to find exactly documenting what something does well when you're reviewing the spectacle of people are trying to interject how they think it should work and illustrates the point where people are debating the issue the the implementation and to to satisfy the continuity of giving you want me to write out to for everything you want to just become this might have been to get that the and I think that's why we've seen a lot of that project move away from this might really detailed spike process the elite that got something more alternative small great
up by the way I just want to ask you something and you stated and I totally agree with you that code review was not it is is not just so 1 important thing but it's just I have a dialog continuous dialog but where do you draw the line sold what we call far do you go with this dialog in order to either rejectable requests are 1st of all requests and so I guess it depends on the situation
you have to really define and explain your project in Europe so maintain of the project and you can simply think do I wanna strange to i want to maintain change in the global and there and working in a company that you probably more thinking along the lines of what is a business need them on a large community it's you thinking about going on your community and think what community want this much of this is the expectation that might not match what you want exactly from a feature that the more there and so that is not really a simple way to do that I think can to yeah make sure you take into account the group you are working with thank you google and
so that's the lunch and don't forget about the size is