Merken

​Keeping Code Style Sanity in a 10-year-old Codebase

Zitierlink des Filmsegments
Embed Code

Automatisierte Medienanalyse

Beta
Erkannte Entitäten
Sprachtranskript
to just a mind and he
might know hi everyone um so let's get started for your last session of the date of deferred to be killed obviously um my name is gabby I
am a developer and the get should than its guilty much of find and this week is kind of special for me because I'm celebrating quite a few things so to their complete of this we can completely 1 year that I've been a top 5 if my one-year anniversary as a random program it also my 1 universe as the rails programmer and it's the 1st time ever that I'm attack conference and it's the 1st time that I'm speaking to attack rock so that basically means that I'm terrified and if I am speaking way too fast the worst thing that can come out of it you be out of here 20 minutes so don't kill me some before we start with the talk I you would like
to clarify a couple things talk about the elephant in the room so this means I'm gonna tell you what I'm not
I'm not here to talk about and I am not here to try and convince you that you should try to implement called style consistency throughout your entire called base so you're wondering what am I here to talk about and I will tell you what the history of trying to tackle this problem at shopify sounds like and will look at things that worked things that didn't work some tools that we're using and in case you do want to tackle that problem in your own code is so that you can 1st the issues before it even come up so let's go back to the beginning if you have seen the talks from my colleague Simon sh and Raphael you probably already know a bit about where it all began while doing the research for this talk I came around our 1st Committee every and get help and I took a screenshot so you can see 2 and he was neighbor of our CEO and founder and it didn't 2005 but then I found out that the committee actually oracle is already existed before it that it started in 2004 but he was an SVN and nodding not using did overdose 13 years of Oracle babies we can notice to things that are really important we have a lot of lines of code here you can see
this table that explain how many lines we have so have a look at it has led to the sinking because of the lot of the the most important thing for us for this talk today is show it to understand how many lines of Ruby code we have because we're going to be talking about specifically about Ruby our Ruby called there's called step we have over 700 thousand and Ruby lines of code to the also we
have a lot of developers are a developer team to be has about the has over 400 developers rails developers working mainly in our shall fight in our court called
this is our what are a rebel looks like after 12 years and we have contour we had contribution for over 700 people from different departments from developers to content as to content writers for example and we're growing
so if you are interested in hearing more about that run job with such a chopper vi clear and back to the top of this what is mean for called style consistency you probably already heard that chopper is 1 off and if not the biggest rubyonrail shop in the world so this means we have we come across some pretty unique problems in our day to day and we have to come up with recreated salt solutions as well the giving 700 line 700 thousand lines of code consistent have been pretty well job but why is that the 1st question right that we had to ask ourselves over the years is do we want to do this do we want to make our Bayes consistent imposing a single called style actually goes kind of against what Ruby their intention of the Ruby language it's right um I came across this interview from map that was done with max and this is what him as a designer of the Ruby language intended for what it is supposed to be a fun language to work with 1 that allows In the user to choose a Walworth best for them and not like other languages that have only 1 way to do things it's supposed to bring creativity and free to choose whatever works best for you at the same time Ruby has changed and evolved so much over those last few years sometimes some styles no longer work and some odds is my username or I show you an example this school this shows you the the outdated hash rocket syntax and he has been mostly replaced by new hashed index both work 1 is newer 1 is older it this brings me to my next
point is shopify was never re written in those last 13 years so this line in this lies that unleash you that all these all files are still there right and advanced we have some old-fashioned corn and at worst it's very inconsistent so let's start into some history here like I mentioned before our
Kate called bees was born in 2004 it was a new app using a new framework it only had a few cock up to people working on it at this point no 1 was really worried what the the cold style and the weather was consistently in whether what they that called was consistent 2005 came about shopify side using get and everyone was working hard to make the app grow and to add more features 2006 came about then 2007 OK and I'm going this er 14 minutes against you get the point by now lesson to 2012 we had a lot more developers our our called was much bigger he was aging and so told hours CEO created a draft style of forgive revealing which in our called is and he had only a half-dozen rules and he was so to service just that a guide for our developers in case you were wondering like what should I write this as 1 the white space is that I'm supposed to use some so they could always refer back to that things got busy I'm not a lot of it was not a lot of attention was but to that issue so this called style conversation was put aside once more this it was brought back up in September 2014 um when we introduce the robocup jam charcoal base to deal with very basic called style rules such as white spaces if you
don't want no Rubalcaba it's age ruby gem it's open source in analyzes the called statically and serves as a linker to follow many of the code style rules that were made by the community for never becoming the the intention was great but
we already had a very quick 1 yeah my thing doesn't work an article it was a kidney mean of this thing blowing up of it we had already a big cold it was 10 years old there had never been linked and so we had many violations of his it is exploited RoboCup because if there was just too many things that needed to be fixed at the same time and so many of those cops word for disabled and we couldn't possibly fix all the violations then how work it doesn't work so instead of running RoboCup through the entire called the if we needed to find another solution so that we could stop the bleeding temporarily and something that would look instead of going through the whole thing it would look only through lines of code that were being changed or newly added so our fellatio jam was came to life palatial or if
you're Brazilian you will call the police out because that's what the name is supposed to be and is a wrapped it's a wrapper built around hold RoboCup that only runs on p ice so no commits and no no not the master branch as Munich accuses calls calls style violations in cold that was added a changed he was inspired in Holland told me by thought but he was actually adopted to what chopper fights needs and constraints were back there of police still
use a you would listen to events of using did have what books and whenever a PR was created by any 1 of our checkout team you would comment on the Pierre with any school style violations that he had introduced the team
was not very impressed the toll was very noisy and he had a lot of comments and appear which is by far the worst common complaint that I've heard while is asking people about that all actually does work and this binding noise they continued using it but that meant that we were in force of some some much needed improvements to the tools that we're using
optomotor thousand 415 was quite uneventful months where Cheney a big change changes came around the first
one he was when we were looking for other solutions without about using the commit status they Gangadhar instead of commenting all over PI to convey const called style violations much like a CI
status the palatial status changes depending on whether you found in violations and if you're not if no violations were found it would be green so that's great you could always merging if you're reviews that's all of NEC did find some violations however it you would turn red and you would you could click in the details but then on the side so they could look at what those violations work this is what our 2
palatial tool looks like some so when you click the detail but it but then you would take it to the stage and you can see all of your style violations the lines were they occur and walk cops and violations a you were you're in French but then this was an opt-in
feature and that developers could choose whether be signed up for it and then had their PIC checked but still even though the tool was an extremely perfect we saw great adoption rate by the beginning of 2016 over 50 % of our dataset already opted-in the 2nd thing that came around at that time um was our shopify Ruby style that he came around together with the absolutely sorry developers could have a better idea of what they called violations word the style violations were and how to fix them what we what was it that we're looking for so this is our style guide
today and it was meant to set expectations for our Ruby developers not only in terms of cold style but also how they are encouraged to take ownership of their own cold and decide for themselves where exceptions to those rules can be reasonable and where the they're allowed to to do it but more important than those expectations the guide offers plenty of examples of what matches our called style and what doesn't so it removes any guessing from when you're developing whether you should you're writing something correctly or not the In my said
a few months later he might 16 we sent out a survey to all or a developer the shop a fight to find out the good the bad and the ugly about using police show in our new style guide that we had implemented so far so some of the questions that we ask the word do you believe has a positive role in our pull requests
the majority of our developers said yes they the real like how the tool was wanted to was doing they believed in what was what he was trying to achieve and those are some
positive feedback that we got that it said it said that it was much easier to on board new people the code was more consistent it said that it allowed called reviews to focus on the called itself rendered in on whether you is there were white spaces in the wrong places and it also said mentioned that he made of a made cold more readable if some common negative feedback revolved around how took longer to get done doesn't make up yeah and then they would push the court to get have and then they would have to fix all of the violations and then they would have to read get feedback from the reviews and England those as well so all they they were not super happy about that because they've increase the lot their work I some of the other common that we got is that sometimes those changes made file less consistent so we reduce readability I show an example of 1 of those cases here if we go back to the hash rocket example of line to is old cold line 3 is something that got newly-added so we have in the same file like very close to each other we have something that is in our old called style and now something that is a new called style that makes the very confusing especially for new developers were starting to learn rails and Ruby and if then we get questions is the use of performance there you the performance difference among those you can I use both the both work and so we wasn't so that he was an ideal the um the next question that we as was did you learn something valuable with palatial and again most people said
yes some of the things that they learn is
syntax improvements things begin know um style best practices in the Ruby community and he was now easier for them to write cold coming from non from non Ruby backgrounds some perfect back that we got some negative feedback that we got was that review on new for me so I know all this stuff ready and I can make those decisions for myself I don't need this still making unified helping me decide this and it also doesn't provide much insight into wider rules exist and how the improve our these next question was those pollution make your life easier on again most people
said yes and because
he was making a call Bismarck predictable so they could better understand it and you would help developers and not have to worry about whether this putting the white space in this line and that line some it because you would tell them when it was some people would been like it who said that there were things that they could improve say and then they usually had to spend more time in their call so they could fix those things and the rules of relations were not always super clear and there where it was annoying the different projects didn't use fellatio and as such they would not have the same cold style so 1 of the cool 1 of the miracle of feedback that we got is that we should please show all the projects In shop defined n del on we knew all at this point that these were not just opinions they were not he said she said kind of opinions they were things that real developers who really use the tools were saying so that we can in 1 way see what worked and in another in an hour sigh advocacy what didn't work so what could we do to improve those tools With such a great response we decided let's pull the shoulder peers then and we went ahead we activated palatial for all per request there were being open initial faculties other apps could also we we also allow other acts to sign up for the service and they could just include that in their own projects an important note here is that given the not so perfect nature of the tools that were being used at the time of Galicia was set up so that you wouldn't feel of when you would entail a build when a the up yeah when they were called violations so the act this is ingredient because in 1 way people could use merge PR is even though there were violations but in another way we could see as a a positive thing because there if there were any emergencies that happen that we need to make the odds are fast so we could choose to merge them even though deliver violations if our tools right science improve a
little bit so in people were getting acquainted with fully show and RoboCup of so we took the efforts and steps for the 1st we included Rubalcaba to run in our Seattle so it was only in 1 component so in most of our check out files and RoboCup we ran through it fix all the violations and then at from then on your build would actually fail if you if you add a new violations the 2nd thing is that we decided she is told the tall so we had to improve other parts of our development Back then and are real scene was a prevailing rail Cyrill's 5 . and 1 of Asia's they were having is that people were adding as we deleted some deprecation the wording new once and the that were not exactly related to called style so we added build some custom cops to prevent people to add those interpretation here you can see an example
of 1 of them on this is this 1 checks if behaviors were being added directly to framework classes but instead you should they should be actually using their own world who provided by the Rails framework this cop was out a correctable so he was sober useful you could just around the RoboCup Dutch 18 you would fix all of those for you the with
answer success was selling the check team we were ready we were ready to roll it out to the entire called running all of Rubalcaba through everything getting rid of all the violations that we currently had we have the perfect plan it was amazing step 1 we would run so all delta correctable cops through the entire code base we did it was awesome
step 2 we would decide on what was the best time to merge those PS so we were looking for a development downtime of to make this very to minimize conflict between our PR is and whatever people were doing at the time um so we chose the weekend because it's a time when not many people were working and there and there's not much going not step number 3 we would send out an e-mail at the beginning of the week on Monday to all the developers to let them know this was happening to get everybody excited but mostly together people to prepare and if they had pull request that they would try their best to merge them by Friday so that on Monday didn't have a lot of conflicts on their on periods and that would have not been so great so we did all about this suffered minimal next we can came about they're like yes let's merge them all In have and called the
it's OK just getting we did not why not it
was a perfect plan widening we go and follow through with it we could we came across some very important blockers so you would break guaranteed history 1 then what does that mean blames would be harder to do now because they're called style 6 commits would not pollute our history by adding things that were not related to the functionality that they're called will strive to to implement and and the shop fight we have the we have a very important value that talks about ownership that means that wherever called that you touch an called leaves you want it now and Back then 1 person was running through all the workouts in a person was going to merge them so now they would be responsible for most of our show of our shopify conveys because and then people get kingdom slack about all of the changes and changes that they might not exactly know that were completely unrelated to do jobs the last thing we came to a very real understanding of what a week at shopify it really is Back then so we were
deploying about 30 times and um so weights time was enough for a massive amount of called to have changed and be newly added some soul that GI that we had prepared a week before were actually all outdated and conflicting with the new call that we had during this week we were really not ready for
this so we went ahead and close up all
those yards and they show that we were using to track those end of
because our problems were all still there so we could we were really not ready for this of pollution was working working the call was incrementally becoming more consistent but by no means word the tools that we're using making developers life easier some of nations were like in the feedback forms that we have we have received and we do some of those things came up right uh so commerce com controversial rules we realized that we were enforcing rules that were either unclear or didn't seem beneficial for companies all cops firm from book were enabled by default and we were just removing the ones that we actually didn't want rails change something and in June last year to actually not do that and the opposite thing to minimize our word our of the frustration among our dads we instead fall rails steps in instead of enabling all cost by default and removing the ones we didn't want We would start with 0 and only at the ones that we actually wanted because that's more Ruby verb like I would say that this strategy actually allowed us to discuss whether we should use the we should add new on a new copper when it surfaced and and to stop discussing things that we're not exactly useful for our base and so getting old getting those cops out in general and this year did
have added a feature that allows you to easily look at the history of a single line and that came to be a very important feature in the lattice story next right so if you don't
know this featured that's what it does and you can click on the little but then and you will allow you to see to go back on the other commits so we
have a few months has that had passed an N a new strategy for cops word quot quite well from our were more receptive to the idea of having a cold style uh guide and and they were confident that while the journey was in great so far we were aiming to improve it so that life would be easier the journey wasn't all rambles and butterflies we appreciate that so but we weren't really getting to it a better place so we would we decided we were ready to try it again in use in RoboCup Celtic right feature to fix all of our of developed violations during tired called base but this time we were using a about so nobody would get bonded in in slack anymore this is what about looks
like when you was raging those ends further violations that we were
of correcting like assigned in the screenshot before we were running then we added that they're out the correct feature to our internal development tool so that people instead of pushing PR and waiting for for each accused for new violations they could actually use our internal poll to fix those before it even push the call also we added those as soon as we merge those be with fixing all of those violations we now added those cops are a CI so our test suit would fail whenever a new violations were added this is nation that we were
using to keep track of all the cops and we we we had a radius used cylindrical base but of course not
everything is perfect right so we had a massive issue 1 of the biggest issues that we found out was when we were adding those cops hurt to our tested and links in the core the entire Cold maize was taking a lot more time than ours slowest test that was in great developers were not going to be happy about this we knew iterating so we we try to find several ways to fix that and temporarily what we found is that we had some giant files in migration files and so we just excluded those from the files to be linked and and that helped a lot the time that he would take to link the entire code base it's by no means a long-term solution tho I'm in a process also 7 this month alone in we had a great responses from that so some of our components and are called the is like checkout customers and gift cards had been completely intervene and I now being fully checked the build time so I want to send some
of your TA teams will actually took on in pushed through the hard times and use those tools and gave was very impressive feedback on how can we improve them and what they didn't like and what we did like so we're gonna talk quickly about where we are
today and where we need to get to so to date like I mentioned we had we have some cops that will run with palatial and some cops they're running at build time so we have 2 ruble copy Emil files to be and so that's not ideal and that we use in the future we want to go back to having just 1 so fixing all the violations I have to date moving those cops inch are tested and then having them run only once something else that developers were really not happy about is that are pushing their called they had to wait for the whole thing to run so that they would see well many violations there were and so on and we want to have Osirak enabled by default so instead of feeling here build you would actually whenever you push PIR you will run through the entire called babies you will make a comment that fixes all of your violations in 2 year open care and then you would run through the test set so then we wouldn't have to worry about it at all um like I mentioned some years ago we had issues with our revealing there's taking too long and so fixing this is in going to be easy we have a solution right now but it's not ideal and will have to be improved so we'll have to use caching um to be able to fix the problem in the long term specially as we add Merkel taco base a lot of the issues we encounter I new and well tackled them and is but also in preference to push em upstream so that the community can also benefit from those improved tools and 1 important thing is that we want to continue improving our style guide because he gives more context and clarity behind why a cop exists wide how does it benefit Our called leaves and readability and all of other and positive things that come out of having 1 very consistent 5 soul let's talk about some final take away that we took from this 13 year giant journey some of the most important thing and this is the thing that you want to take back with you is that do you need to have whenever you're thinking about this effort you need to have a place for discussion developers are very opinionated alot of the times and they would like to sometimes discussed whether this is good odd this is not something my not my work for you but it might not work for a very large number of people so we want to have a place where they can actually chat about and bring new ideas and new ideas which brings me to my next point pick styles that work for a cold base in many places RoboCup is not very opinionated about what should should your style would be like and you can pick 1 of the many that the offer and you can talk to other people in your team to decide what's the best 1 for you so
here is 1 example of something that is good and good so you can just pick 1 the top to the people that work with you and decide what's the best 1 for you all 4 roles
to be challenged its Mahler the same thing but just because it's very it doesn't mean that it should be there for the rest of the time so if it's not working discuss whether you should remove it or not it's all about timing you're tools have to be someone prepared a new developers need to be at least partially own board for this change should be successful so but don't give up that hard times it's not going to be any the journey of the time and never going to be absolutely perfect choose a term is good enough but don't even up talk to others to learn more about it tools so that you know what can and can't be done tackle 1 thing at a time but the most important thing you have fun through the process I've learned a lot through it working tackle trying to tackle this problem a chopper fight I met a lot of some people and so can you thank you so much the thank you for that so if the
if we were
Programmiergerät
App <Programm>
Maschinencode
Softwareentwicklung
Softwareentwickler
Grundraum
Computeranimation
Torus
Bit
Maschinencode
Widerspruchsfreiheit
Gerade
Computeranimation
Softwareentwickler
Maschinencode
Task
Softwaretest
Einheit <Mathematik>
Objektklasse
Softwareentwickler
Gerade
Gerade
Computeranimation
Tabelle <Informatik>
Subtraktion
Maschinencode
Extrempunkt
Formale Sprache
Snake <Bildverarbeitung>
Indexberechnung
Widerspruchsfreiheit
Computeranimation
Mapping <Computergraphik>
Rechter Winkel
Prozess <Informatik>
Automatische Indexierung
Maschinencode
Hash-Algorithmus
Inhalt <Mathematik>
Softwareentwickler
Brennen <Datenverarbeitung>
Widerspruchsfreiheit
Gerade
App <Programm>
Dienst <Informatik>
Umsetzung <Informatik>
Punkt
Schlussregel
Elektronischer Programmführer
Softwareentwickler
Elektronische Publikation
Widerspruchsfreiheit
Framework <Informatik>
Raum-Zeit
Gerade
Computeranimation
Arithmetisches Mittel
Maschinencode
Open Source
Programmierstil
Wort <Informatik>
Schlussregel
Binder <Informatik>
Gerade
Computeranimation
Nebenbedingung
Wrapper <Programmierung>
Verzweigendes Programm
Systemaufruf
Ereignishorizont
Computeranimation
Forcing
Mathematisierung
Geräusch
Computeranimation
Drucksondierung
Mathematisierung
Computeranimation
Wort <Informatik>
Elektronischer Programmführer
Bitrate
Softwareentwickler
Bitrate
Gerade
Computeranimation
Erwartungswert
Wort <Informatik>
Ausnahmebehandlung
Schlussregel
Elektronischer Programmführer
Sondierung
Softwareentwickler
Term
Computeranimation
Rückkopplung
Subtraktion
Maschinencode
Mathematisierung
Hash-Algorithmus
Elektronische Publikation
Softwareentwickler
Whiteboard
Raum-Zeit
Gerade
Computeranimation
Videospiel
Schlussregel
Computeranimation
Entscheidungstheorie
App <Programm>
Rückkopplung
Punkt
Natürliche Zahl
Fakultät <Mathematik>
Relativitätstheorie
Systemaufruf
Schlussregel
Raum-Zeit
Computeranimation
Dienst <Informatik>
Prognoseverfahren
Offene Menge
Rechter Winkel
Endogene Variable
Projektive Ebene
Softwareentwickler
Brennen <Datenverarbeitung>
Gerade
Demoszene <Programmierung>
Interpretierer
Bit
Klasse <Mathematik>
Mereologie
Wort <Informatik>
Zusammenhängender Graph
Softwareentwickler
Elektronische Publikation
Framework <Informatik>
Computeranimation
Maschinencode
Automatische Handlungsplanung
Ganze Funktion
Computeranimation
Versionsverwaltung
Zahlenbereich
Softwareentwickler
E-Mail
Frequenz
Computeranimation
Lineares Funktional
Gewicht <Mathematik>
Prozess <Informatik>
Maschinencode
Mathematisierung
Automatische Handlungsplanung
Systemaufruf
Computeranimation
Rückkopplung
Videospiel
Wiki
Default
Mathematisierung
Systemaufruf
Schlussregel
Extrempunkt
Computeranimation
Schlussregel
Eins
Arithmetisches Mittel
Bildschirmmaske
Einheit <Mathematik>
Geometrische Frustration
Strategisches Spiel
COM
Flächeninhalt
Wort <Informatik>
Softwareentwickler
Default
Videospiel
Verbandstheorie
Rechter Winkel
Strategisches Spiel
Wort <Informatik>
Computeranimation
Softwaretest
Suite <Programmpaket>
Systemaufruf
Softwareentwickler
Computeranimation
Softwaretest
Radius
Maschinencode
Prozess <Physik>
Elektronische Publikation
Binder <Informatik>
Marketinginformationssystem
Computeranimation
Chipkarte
Komponente <Software>
Weg <Topologie>
Rechter Winkel
Migration <Informatik>
Endogene Variable
Speicherabzug
Zusammenhängender Graph
Softwareentwickler
Ganze Funktion
Kreiszylinder
Softwaretest
Rückkopplung
Elektronische Publikation
Punkt
Kontrollstruktur
Offene Menge
Zahlenbereich
Elektronischer Programmführer
Kontextbezogenes System
Softwareentwickler
Term
Default
Computeranimation
Arithmetisches Mittel
Prozess <Physik>
Raum-Zeit
Mathematisierung
Softwareentwickler
Term
Whiteboard
Computeranimation
Schlussregel

Metadaten

Formale Metadaten

Titel ​Keeping Code Style Sanity in a 10-year-old Codebase
Serientitel RailsConf 2017
Teil 78
Anzahl der Teile 86
Autor Stefanini, Gabi
Lizenz CC-Namensnennung - Weitergabe unter gleichen Bedingungen 3.0 Unported:
Sie dürfen das Werk bzw. den Inhalt zu jedem legalen und nicht-kommerziellen Zweck nutzen, verändern und in unveränderter oder veränderter Form vervielfältigen, verbreiten und öffentlich zugänglich machen, sofern Sie den Namen des Autors/Rechteinhabers in der von ihm festgelegten Weise nennen und das Werk bzw. diesen Inhalt auch in veränderter Form nur unter den Bedingungen dieser Lizenz weitergeben.
DOI 10.5446/31255
Herausgeber Confreaks, LLC
Erscheinungsjahr 2017
Sprache Englisch

Inhaltliche Metadaten

Fachgebiet Informatik
Abstract This is a sponsored talk by Shopify. Conversations around code consistency seem to spark either cheers or jeers from developers. In this talk, I'll explore the good, bad, and the ugly of code style consistency as illustrated by the (sometimes drama-filled) history of Shopify's 10-year-old codebase. Highlighting strategies to help you evaluate when to push for better code consistency; you will hear about our techniques, tools and guides to enrich developer experience without compromising productivity and how to ultimately make code consistency important across the organization.

Ähnliche Filme

Loading...