Effective Code Review
Formal Metadata
| Title |
Effective Code Review
|
| Title of Series | |
| Part Number |
44
|
| Number of Parts |
169
|
| Author |
|
| License |
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. |
| Identifiers |
|
| Publisher |
|
| Release Date |
2016
|
| Language |
English
|
Content Metadata
| Subject Area | |
| Abstract |
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
|
00:00
Wiki
Context awareness
Roundness (object)
Red Hat
Computer animation
Lecture/Conference
View (database)
Multiplication sign
Machine code
Machine code
00:41
Integrated development environment
Multiplication sign
Quicksort
01:18
Computer animation
Link (knot theory)
Dualism
Machine code
Chord (peer-to-peer)
Twitter
02:02
Mathematics
Lecture/Conference
Semiconductor memory
Personal digital assistant
Projective plane
Videoconferencing
Machine code
Computer programming
Number
02:44
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)
03:58
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)
04:48
Word
Process (computing)
Computer animation
Software developer
Meeting/Interview
Multiplication sign
Software developer
Machine code
Average
Machine code
Vector potential
Number
05:36
Expected value
Process (computing)
Red Hat
Computer animation
Link (knot theory)
Lecture/Conference
Shared memory
Sound effect
Mereology
Machine code
Computer programming
Number
06:26
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
07:33
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
08:21
Point (geometry)
Functional (mathematics)
Computer animation
Lecture/Conference
Connectivity (graph theory)
Telecommunication
Multiplication sign
Projective plane
Software testing
Heat transfer
Machine code
Machine code
08:57
Data mining
Computer animation
Lecture/Conference
View (database)
Mereology
Local ring
09:33
Point (geometry)
Collaborationism
Computer animation
View (database)
Core dump
Collaborationism
Disk read-and-write head
Machine code
10:14
Revision control
Collaborationism
Dependent and independent variables
Mathematics
Process (computing)
Computer animation
Lecture/Conference
Multiplication sign
1 (number)
Bit
Figurate number
Quicksort
10:49
Process (computing)
Computer animation
Open source
Projective plane
11:23
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
12:12
Context awareness
Context awareness
Lecture/Conference
Meeting/Interview
Software developer
Projective plane
1 (number)
Software testing
Machine code
Computing platform
12:53
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
13:33
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)
14:11
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
15:00
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
15:40
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
17:10
Point (geometry)
Computer animation
Lecture/Conference
Feedback
Machine code
Neuroinformatik
17:52
Dependent and independent variables
Computer animation
Lecture/Conference
Projective plane
Shared memory
Authorization
Mereology
18:27
Mathematics
Word
Dependent and independent variables
Meeting/Interview
Projective plane
Authorization
Machine code
Resultant
19:04
Category of being
Mathematics
Red Hat
Meeting/Interview
Lecture/Conference
Forcing (mathematics)
Projective plane
Quicksort
Lattice (order)
Computer programming
19:44
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
20:59
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
22:09
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
23:26
Point (geometry)
Computer animation
Information
Electronic program guide
Extension (kinesiology)
Disk read-and-write head
24:03
Slide rule
Multiplication
Mathematics
Computer animation
Lecture/Conference
View (database)
Mehrplatzsystem
Source code
Maxima and minima
24:39
Game controller
Arm
Software
Meeting/Interview
Lecture/Conference
Machine code
Line (geometry)
Distance
25:22
Computer animation
Lecture/Conference
Moment (mathematics)
Point (geometry)
Feedback
25:56
Computer animation
Lecture/Conference
Moment (mathematics)
Point (geometry)
Machine code
Traffic reporting
26:31
Point (geometry)
Computer animation
Negative number
Line (geometry)
27:07
Dynamical system
Data management
Integrated development environment
Open source
Lecture/Conference
Meeting/Interview
Personal digital assistant
Speech synthesis
Distance
27:49
Computer animation
Query language
Collaborationism
Authorization
Machine code
Data conversion
Thermal conductivity
Wave packet
28:35
Open source
Multiplication sign
Feedback
Projective plane
Collaborationism
Shared memory
Product (business)
Word
Mathematics
Computer animation
Lecture/Conference
Form (programming)
Social class
29:27
Point (geometry)
Touchscreen
Computer animation
30:02
Computer animation
Lecture/Conference
Universe (mathematics)
Feedback
Water vapor
Traffic reporting
30:40
Constraint (mathematics)
Computer animation
Source code
Summierbarkeit
31:19
Type theory
Computer animation
Open source
Lecture/Conference
Logic
Feedback
Basis <Mathematik>
Rule of inference
32:17
Point (geometry)
Word
Computer animation
Information
Feedback
Projective plane
Machine code
Spectrum (functional analysis)
Number
32:54
Type theory
Red Hat
Computer animation
Lecture/Conference
Coma Berenices
Spacetime
Stack (abstract data type)
Quicksort
Open set
Twitter
Spacetime
33:37
Collaborationism
Projective plane
Machine code
34:40
Statistical hypothesis testing
Type theory
Message passing
Commitment scheme
Computer file
Term (mathematics)
Projective plane
35:33
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
37:08
Point (geometry)
Touchscreen
Multiplication sign
Projective plane
Computer programming
Type theory
Radical (chemistry)
Mathematics
Process (computing)
Meeting/Interview
Universe (mathematics)
Metropolitan area network
Resultant
38:30
Axiom of choice
Point (geometry)
Implementation
Dependent and independent variables
Red Hat
Software developer
Feedback
Energy level
Machine code
Lattice (order)
Arithmetic progression
40:01
Implementation
Mathematics
Red Hat
State of matter
Average
Software developer
Right angle
Branch (computer science)
Machine code
Line (geometry)
Hand fan
41:22
Point (geometry)
Implementation
Red Hat
Multiplication sign
Projective plane
Feedback
Bit
Machine code
Mathematics
Latent heat
Process (computing)
Repository (publishing)
Analytic continuation
43:04
Expected value
Mathematics
Group action
Meeting/Interview
Order (biology)
Projective plane
Machine code
Line (geometry)
Analytic continuation
Software maintenance
00:00
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
00:14
that will 1st of all thanks coming and for having me here having thinking about what could
00:18
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
00:33
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
00:42
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
00:56
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
01:19
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
01:29
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
01:43
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
01:51
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
02:03
at work and by that I mean that have the overall the review of a change the things that a good percentage of
02:09
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
02:25
program you that looks forward to in
02:29
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
02:45
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
03:25
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
03:59
project the unification when you look for Western Europe was and so 1st of the support from all of
04:07
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
04:28
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
04:49
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
05:08
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
05:20
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
05:37
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
05:48
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
06:03
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
06:27
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
06:46
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
06:56
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
07:34
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
08:00
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
08:22
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
08:34
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
08:52
rated higher than the so was the of
08:59
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
09:08
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
09:34
core deposits comes along is almost an adversarial like you working against each other rather
09:38
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
09:52
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
10:01
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
10:14
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
10:40
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
10:51
can begin to use which is useful this first one I think is really really
10:58
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
11:24
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
11:53
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
12:13
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
12:26
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
12:39
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
12:54
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
13:13
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
13:34
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
13:48
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
13:59
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
14:11
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
14:29
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
15:01
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
15:25
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
15:40
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
16:40
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
17:11
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
17:28
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
17:40
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
17:57
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
18:09
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
18:30
see the reviewers of sorry the reviewer of code over the years
18:34
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
19:05
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
19:21
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
19:45
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
20:25
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
21:02
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
21:48
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
22:09
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
22:28
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
22:41
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
23:27
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
23:39
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
24:05
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
24:18
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
24:40
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
24:52
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
25:23
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
25:36
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
25:56
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
26:33
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
26:58
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
27:09
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
27:40
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
27:51
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
28:13
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
28:36
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
28:50
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
29:05
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
29:28
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
29:48
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
30:03
to master the of water in reports and make sure that you are having some review before that
30:09
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
30:40
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
30:53
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
31:19
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
31:38
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
32:17
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
32:28
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
32:54
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
33:11
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
33:35
wanting to know if you're looking at that
33:37
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
34:08
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
34:46
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
35:07
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
35:38
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
36:29
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
37:08
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
37:34
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
38:45
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
39:27
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
40:06
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
40:21
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
41:36
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
42:02
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
43:05
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
43:33
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
44:13
so that's the lunch and don't forget about the size is
