​Keeping Code Style Sanity in a 10-year-old Codebase
CC Attribution - 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
Stefanini, Gabi
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.
 Hi, everyone. So let's get started for your last session of the day before the big keynote, obviously. My name is Gabi. I am a developer at the Get-Sheet-Dunnet scale team at Shopify. And this week is kind of special for me because I'm celebrating quite a few things. So today I complete, this week I'm completing one year that I've been at Shopify. It's my one year anniversary as a Ruby programmer. It's also my one year anniversary as a Rails programmer. It's the first time ever that I'm at a tech conference. And it's the first time that I'm speaking to a tech realm. So that basically means that I'm terrified. And if I am speaking way too fast, the worst thing that could come out of it, you'll be out of here 20 minutes early. So don't kill me. Before we start with the talk, I would like to clarify a couple of things. Talk about the elephant in the room. So this means I'm going to tell you what I'm not here to talk about. I am not here to try and convince you that you should try to implement code-style consistency throughout your entire code base. So you're wondering what am I here to talk about? I will tell you what the history of trying to tackle this problem at Shopify sounds like. And we'll 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 base, that you can foresee issues before they even come up. So let's go back to the beginning. If you have seen the talks from my colleague Simon and Rafael, you probably already know a bit about where it all began. While doing the research for this talk, I came around our first commit ever in GitHub, and I took a screenshot so you can see it too. It was made by our CEO and founder, Toby. It dated 2005, but then I found out that the commit actually, our code base already existed before that. It started in 2004, but it was in SVN and not using Git. Over the 13 years of our code base, we can notice two things that are really important. We have a lot of lines of code. Here you can see this table that explains how many lines we have, so I'll have a look at it. Let us sink in, because it's a lot of code. The most important thing for this talk today is to understand how many lines of Ruby code we have, because we're going to be talking specifically about our Ruby code style. We have over 700,000 Ruby lines of code today. Also we have a lot of developers. Our developer team today has over 400 developers, Rails developers, working mainly in our Shopify, in our core code base. This is what our repo looks like after 12 years. We had contribution for over 700 people from different departments, from developers to content writers, for example. Then we're growing. If you are interested in hearing more about that, run to our booth at Shopify later on. Back to the talk. What does this mean for code style consistency? You probably already heard that Shopify is one of, and if not the biggest, Ruby on Rails shop in the world. This means we come across some pretty unique problems in our day-to-day, and we have to come up with pretty creative solutions as well. Keeping 700,000 lines of code consistent have been a pretty well job, but why is that? The first question that we had to ask ourselves over these years is, do we want to do this? Do we want to make our code base consistent? Creating a single code style actually goes against what the intention of the Ruby language is. I came across this interview from Matt that was done with Matt. This is what him, as the designer of the Ruby language, intended for it. It is supposed to be a fun language to work with. And it allows the user to choose what works best for them, and not like other languages that have only one way to do things. It's supposed to bring creativity and for you 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 are just not used anymore. I'll show you one example. This shows you the outdated hash rocket syntax. It has been mostly replaced by the newest hash syntax. Both work. One is newer. One is older. This brings me to my next point, which is Shopify was never rewritten in those last 13 years. So this lies in an issue that all these outdated styles are still there. At best, we have some old-fashioned code, and at worst, it's very inconsistent. So let's dive into some history here. Like I mentioned before, our code base was born in 2004. It was a new app using a new framework. It only had a few people working on it. At this point, no one was really worried about the code style and whether it was consistent. If the code base was consistent. 2005 came about. Shopify started using it, and everyone was working hard to make the app grow and to add more features. 2006 came about. Then 2007. Okay, I'm not going to do this for 40 minutes. I guess you get the point by now. Let's jump to 2012. We had a lot more developers. Our code base was much bigger. It was aging. So Toby, our CEO, created a draft style for Ruby language in our code base. He had only a half dozen rules, and he was to serve as just a guide for our developers in case they were wondering what should I write this as, what are the white spaces that I'm supposed to use so they could always refer back to that. Things got busy. Not a lot of attention was put to that issue. So this code style conversation was put aside once more. It was brought back up in September 2014 when we introduced the Rubocop gem to our code base to deal with very basic code style rules such as white spaces. If you don't yet know Rubocop, it's a Ruby gem. It's open source, and it analyzes your code statically and serves as a linter to follow many of the code style rules that were made by the community for the Ruby community. The intention was great. But we already had a very, wait, one second. My thing doesn't work. I don't know how to make it work. It was a cute meme of this thing blowing up. We had already a big code base. It was 10 years old that had never been linted, so we had many violations. Obviously, this exploded Rubocop because there was just too many things that needed to be fixed at the same time. So many of those cops were disabled, and we couldn't possibly fix all of the violations then. Now it will work? No. It doesn't want to work. So instead of running Rubocop through the entire code base, we needed to find another solution so that we could stop the bleeding temporarily. 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 police show jam came to life. Police show, or if you're a Brazilian, you will call it policial because that's what the name is supposed to be. It's a wrapper built around Rubocop that only runs on PRs, so no commits and no, not the master branch. We need to accuse this code-style violations in code that was added or changed. It was inspired in the Houndtool made by Thoughtbot, but it was actually adapted to what Shopify's needs and constraints were back then. Policial used, it listened to events using GitHub webhooks, and whenever a PR was created by any one of our checkout team, it would comment on the PR with any code-style violations that they had introduced. The team was not very impressed. The tool was very noisy, and it added a lot of comments in the PR, which is by far the worst common complaint that I've heard while I was asking people about that. Oh, it actually does work. Despite the noise, they continued using it, but that meant that we were in for some much-needed improvements to the tools that we were using. October 2015 was quite an eventful month, where two new big changes came around. The first one, it was when we were looking for other solutions, we thought about using the commit status API in GitHub instead of commenting all over the PR to convey code-style violations. Much like a CI status, the police status changes depending on whether you found any violations on the PR or not. If no violations were found, it would be green, so that's great. You could always merge if your reviews said so. If you did find some violations, however, it would turn red, and you could click in the details button on the side so that you could look at what those violations were. This is what our Polatio tool looks like. When you click the detail button, it would take you to this page. You can see all of your style violations, the lines where they occur, and what cops and violations you are infringing. Back then, this was an opt-in feature that developers could choose whether they signed up for it and then had their PRs checked by this tool. Even though the tool wasn't extremely perfect, we saw a great adoption rate. By the beginning of 2016, over 50% of our devs had already opted in. The second thing that came around at that time was our Shopify Ruby style guide. It came around together with the app so our developers could have a better idea of what the code violations were, the style violations were, and how to fix them. What was it that we were looking for? This is our style guide today. It was meant to set expectations for our Ruby developers, not only in terms of code style, but also how they're encouraged to take ownership of their own code and decide for themselves where exceptions to those rules can be reasonable and where they are allowed to do it. But more important than those expectations, the guide offers plenty of examples of what matches our code style and what doesn't. It removes any guessing from when you're developing whether you are writing something correctly or not. A few months later, in March 2016, we sent out a survey to all our developers at Shopify to find out the good, the bad, and the ugly about using PoliShow and our new style guide that we had implemented so far. Some of the questions that we asked were, do you believe PoliShow has a positive role in our pull requests? The majority of our developers said yes. They really liked what the tool was doing. They believed in what it was trying to achieve. Those are some positive feedback that we got. It said that it was much easier to onboard new people. Now the code was more consistent. It said that it allowed code reviews to focus on the code itself, whether it was there were white spaces in the wrong places. It also mentioned that it made code more readable. Some negative feedback revolved around how PRs took longer to get done. Devs would make a PR and then they would push their code to GitHub and then they would have to fix all of the violations. Then they would have to get feedback from their reviews and implement those as well. So they were not super happy about that because it increased a lot their work. The other comment that we got is that sometimes those changes made file less consistent so it reduced readability. I'll show you an example of one of those cases. Here if we go back to the hash rocket example, line two is old code. Line three is something that got newly added. We have in the same file very close to each other. We have something that is in our old code style and now something that is in new code style. That makes it very confusing. Especially for new developers who are starting to learn Rails and Ruby. Then we get questions of is there a performance difference among those two? Can I use both? Do both work? So it wasn't ideal. The next question that we asked was did you learn something valuable with Poly Show? And again, most people said yes. Some of the things that they learned is syntax improvements, things they didn't know, style best practices in the Ruby community. And it was easier for them to write code coming from non-Ruby backgrounds. Some poor feedback that we got, some negative feedback that we got was that Ruby is not new for me so I know all this stuff already and I can make those decisions for myself. I don't need this tool making me decide, helping me decide this. It also doesn't provide much insight into why the rules exist and how they improve our code base. Next question was does Poly Show make your life easier? Again, most people said yes and because it was making the code base more predictable so they could better understand it and it would help developers not have to worry about whether putting the white space in this line or in that line because you would tell them when it was wrong. Some people who didn't like it or said there were things they could improve said they usually had to spend more time in their code so they could fix those things. The rules of violations were not always super clear and it was annoying that different projects didn't use Poly Show and as such they would not have the same code style. So one of the cool feedback that we got is that we should show all the projects in Shopify and we knew at this point that these were not just opinions. They were not, she said kind of opinions. They were things that real developers who really use the tools were saying so that we can in one way see what worked and in another side we could see what didn't work so what could we do to improve those tools. With such a great response we decided let's Poly Show all the PRs then. We went ahead, we activated Poly Show for all the requests that were being opened in the Shopify code base. Other apps could also, we also allowed other apps to sign up for the service and they could 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, Poly Show was set up so that it wouldn't fail when it wouldn't fail a build when there were code violations to a PR. This isn't great because in one way people could just merge PRs even though there were violations but in another way we could see as a positive thing because if there were any emergencies that happened that we needed to merge PRs very fast we could choose to merge them even though there were violations. Our tools started to improve a little bit so people were getting acquainted with Poly Show and RuboCop so we took the effort some steps further. First we included RuboCop to run in our CI so it was only in one component so in most of our checkout files RuboCop we ran through it, fixed all the violations and then from then on your build would actually fail if you added new violations. The second thing is that we decided to use tools, the tools that we had to improve other parts of our development. Back then our Rails team was upgrading Rails to Rails 5.0 and one of the issues they were having is that people were adding, as we deleted some deprecations, new ones that were not exactly related to cold style so we added some custom cops to prevent people to add those new deprecations. Here you can see an example of one of them. This one checks if behaviors were being added directly to framework classes but instead they should be actually using the onload hook provided by the Rails framework. This cop was auto correctable so it was super useful. You could just run the RuboCop dash A and it would fix all of those for you. With its success we saw in the checkout team we were ready to roll it out to the entire code base running all of RuboCop through everything getting rid of all the violations that we currently had. We had the perfect plan. It was amazing. Step one, we would run all the auto correctable cops through the entire code base. We did it. It was awesome. Step two, we would decide on what was the best time to merge those PRs. So we were looking for a development downtime to minimize conflict between our PRs and whatever people were doing at the time. So we chose the weekend because it's a time when not many people are working and there's not much going on. Step number three, we would send out an email at the beginning of the week on Monday to all of the developers to let them know this was happening, to get everybody excited but mostly to get people to prepare and if they had open pull requests that they would try their best to merge them by Friday so that on Monday they didn't have a lot of conflicts on their own PRs and that would have not been so great. So we did all that except for the email. Next weekend came about and we were like, yes, let's merge them all and have a LinkedIn code base. Okay, just kidding. We did not. Why not? It was the perfect plan. Why didn't we go and follow through with it? We came across some very important blockers. So it would break our Git history. What does that mean? Blames would be harder to do now because the code style fix commits would now pollute our history by adding things that were not related to the functionality that the code was trying to implement. And the Shopify, we have a very important value that talks about ownership. That means that wherever code that you touch in our code base, you want it now. And back then, one person was running through all of the Ruby cops and a person was going to merge them. So now they would be responsible for most of our Shopify code base and then people get pinged on Slack about all of the changes and changes that they might not exactly know that were completely unrelated to their jobs. The last thing, we came to a very real understanding of what a week at Shopify really is. Back then, we were deploying about 30 times a day. So a week's time was enough for a massive amount of code to have changed and been newly added. So the PRs that we had prepared the week before were actually all outdated and conflicting with the new code that we had added during this week. We were really not ready for this. So we went ahead and closed up all those PRs and the issue that we were using to track those. And because our problems were all still there, so we were really not ready for this. Policia was working. The code was incrementally becoming more consistent, but by no means were the tools that we were using making developers' lives easier. Some of the issues were, like in the feedback forms that we have received, some of those things came up. So controversial rules. We realized that we were enforcing rules that were either unclear or that didn't seem beneficial for our code base. All cops from Rubikop were enabled by default, and we were just removing the ones that we actually didn't want. Rails changed something in June last year to actually not do that and do the opposite thing, to minimize our frustration among our devs, we instead follow Rails steps, and instead of enabling all cops by default and removing the ones we didn't want, we would start with zero and only add the ones that we actually wanted, because that's more Ruby. Ruby-like, I would say. This new strategy actually allowed us to discuss whether we should use, we should add a new cop when it surfaced, and to stop discussing things that were not exactly useful for our code base. And so getting those cops out. In January this year, GitHub added an update feature that allows you to easily look at the history of a single line, and that came to be a very important feature that led us to our next try. So if you don't know this feature, that's what it does. You can click on the little button and it will allow you to see, to go back on other commits. So we, a few months had passed, and a new strategy for cops worked quite well. Our devs were more receptive today, the idea of having a code style guide, and they were confident that while the journey wasn't great so far, we were aiming to improve it so that life would be easier. The journey wasn't all rainbows and butterflies, we appreciate that. But we were really getting to a better place. So we decided we were ready to try it again and use RuboCops out of correct feature to fix all of our, of the violations to our entire code base. But this time we were using a bot, so nobody would get bothered in Slack anymore. This is what our bot looks like when it was merging those. And for the violations that we were already correcting, like you saw in the screenshot before, we were running the, we added the out of correct feature to our internal development tool so that people instead of pushing PRs and waiting for those, for it to accuse for new violations, they could actually use our internal tool to fix those before they even push their code. Also, we added, as soon as we merged those PRs with fixing all of those violations, we now added those cops to our CI. So our test suite would fail whenever new violations were added. This is the issue that we were using to keep track of all the cops that we had already used to link our code base. But of course, not everything is perfect, right? So we had a massive issue. One of the biggest issues that we found was when we were adding those cops to our test suite, linking the entire code base was taking a lot more time than our slowest test. That wasn't great. Developers were not going to be happy about this. We knew it already. So we tried to find several ways to fix that, and temporarily what we found is that we had some giant files, migration files, and so we just excluded those from the files to be linked in, and that helped a lot the time that it would take to link the entire code base. It's by no means a long-term solution, though. In April 2007, this month, we had great responses from that, so some of our components in our code base, like checkout, customers and gift cards, had been completely linted and are now being fully checked at build time. So I want to send some love here to our teams who actually took on and pushed through the hard times and used those tools and gave us very impressive feedback on how can we improve them and what they didn't like and what they did like. So we're going to talk quickly about where we are today and where we need to get to. So today, like I mentioned, we have some cops that will run with Polissio and some cops that are running at build time. So we have two RuboCop EML files today, so that's not ideal, and in the future, we want to go back to having just one. So fixing all the violations that we have today, moving those cops into our test suit and then having them run only once. Something else that developers were really not happy about is that pushing their code, they had to wait for the whole thing to run so that they would see how many violations there were and so on. We want to have AutoCorrect enabled by default. So instead of failing your build, it would actually, whenever you push your PR, it will run through your entire code base. It will make a commit that fixes all of your violations into your open PR and then it would run through the test suit. So then we wouldn't have to worry about it at all. Like I mentioned some minutes ago, we had issues with our Ruby linters taking too long. So fixing this isn't going to be easy. We have a solution right now, but it's not ideal, and we'll have to improve, so we'll have to use caching to be able to fix the problem in the long term, especially as we add more code to our code base. A lot of the issues we encounter, I knew, and while we tackle them, it's also important for us to push them upstream so that the community can also benefit from those improved tools. And one important thing is that we want to continue improving our style guide because it gives more context and clarity behind why a cop exists, why, how does it benefit our code base and readability and all of other positive things that come out of having one very consistent file. So let's talk about some final takeaways that we took from this 13-year journey. The most important thing, and this is the thing that you want to take back with you, is that whenever you're thinking about this effort, you need to have a place for discussion. Developers are very opinionated a lot of the times, and they would like to sometimes discuss whether this is good or this is not. It might not work for you, but it might not work for a very large number of people, so you want to have that place where they can actually chat about it and bring new ideas, which brings me to my next point. Pick styles that work for your code base. In many places, RuboCop is not very opinionated about what should your style be like. You can pick one of the many that they offer, and you can talk to other people in your team to decide what's the best one for you. So here is one example of something that is good and good, so you can just pick one, but talk to the people that work with you and decide what's the best one for you. Allow for rules to be challenged. It's more or less the same thing. Just because it's there, 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. Your tools have to be somewhat prepared, and your developers need to be at least partially on board for this change to be successful. But don't give up at hard times. It's not going to be an easy journey. The time is never going to be absolutely perfect. Choose a time when it's good enough, but don't give up. Talk to others. Learn more about your tools so that you know what can and can't be done. Tackle one thing at a time. But the most important thing is have fun through the process. I've learned a lot through working, trying to tackle this problem at Shopify. I met a lot of awesome people, and so can you. Thank you so much. Thanks to the developers for making this opportunity to speak to me. So it's coming.
 Hi, everyone.  So let's get started for your last session of the day before the big keynote, obviously.  My name is Gabi.  I am a developer at the Get-Sheet-Dunnet scale team at Shopify.  And this week is kind of special for me because I'm celebrating quite a few things.  So today I complete, this week I'm completing one year that I've been at Shopify.  It's my one year anniversary as a Ruby programmer.  It's also my one year anniversary as a Rails programmer.  It's the first time ever that I'm at a tech conference.  And it's the first time that I'm speaking to a tech realm.  So that basically means that I'm terrified.  And if I am speaking way too fast, the worst thing that could come out of it, you'll be  out of here 20 minutes early.  So don't kill me.  Before we start with the talk, I would like to clarify a couple of things.  Talk about the elephant in the room.  So this means I'm going to tell you what I'm not here to talk about.  I am not here to try and convince you that you should try to implement code-style consistency  throughout your entire code base.  So you're wondering what am I here to talk about?  I will tell you what the history of trying to tackle this problem at Shopify sounds like.  And we'll 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 base, that you can foresee  issues before they even come up.  So let's go back to the beginning.  If you have seen the talks from my colleague Simon and Rafael, you probably already know  a bit about where it all began.  While doing the research for this talk, I came around our first commit ever in GitHub,  and I took a screenshot so you can see it too.  It was made by our CEO and founder, Toby.  It dated 2005, but then I found out that the commit actually, our code base already existed  before that.  It started in 2004, but it was in SVN and not using Git.  Over the 13 years of our code base, we can notice two things that are really important.  We have a lot of lines of code.  Here you can see this table that explains how many lines we have, so I'll have a look  at it.  Let us sink in, because it's a lot of code.  The most important thing for this talk today is to understand how many lines of Ruby code  we have, because we're going to be talking specifically about our Ruby code style.  We have over 700,000 Ruby lines of code today.  Also we have a lot of developers.  Our developer team today has over 400 developers, Rails developers, working mainly in our Shopify,  in our core code base.  This is what our repo looks like after 12 years.  We had contribution for over 700 people from different departments, from developers to  content writers, for example.  Then we're growing.  If you are interested in hearing more about that, run to our booth at Shopify later on.  Back to the talk.  What does this mean for code style consistency?  You probably already heard that Shopify is one of, and if not the biggest, Ruby on Rails  shop in the world.  This means we come across some pretty unique problems in our day-to-day, and we have to  come up with pretty creative solutions as well.  Keeping 700,000 lines of code consistent have been a pretty well job, but why is that?  The first question that we had to ask ourselves over these years is, do we want to do this?  Do we want to make our code base consistent?  Creating a single code style actually goes against what the intention of the Ruby language  is.  I came across this interview from Matt that was done with Matt.  This is what him, as the designer of the Ruby language, intended for it.  It is supposed to be a fun language to work with.  And it allows the user to choose what works best for them, and not like other languages  that have only one way to do things.  It's supposed to bring creativity and for you 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 are just not used anymore.  I'll show you one example.  This shows you the outdated hash rocket syntax.  It has been mostly replaced by the newest hash syntax.  Both work.  One is newer.  One is older.  This brings me to my next point, which is Shopify was never rewritten in those last 13 years.  So this lies in an issue that all these outdated styles are still there.  At best, we have some old-fashioned code, and at worst, it's very inconsistent.  So let's dive into some history here.  Like I mentioned before, our code base was born in 2004.  It was a new app using a new framework.  It only had a few people working on it.  At this point, no one was really worried about the code style and whether it was consistent.  If the code base was consistent.  2005 came about.  Shopify started using it, and everyone was working hard to make the app grow and to add  more features.  2006 came about.  Then 2007.  Okay, I'm not going to do this for 40 minutes.  I guess you get the point by now.  Let's jump to 2012.  We had a lot more developers.  Our code base was much bigger.  It was aging.  So Toby, our CEO, created a draft style for Ruby language in our code base.  He had only a half dozen rules, and he was to serve as just a guide for our developers  in case they were wondering what should I write this as, what are the white spaces that  I'm supposed to use so they could always refer back to that.  Things got busy.  Not a lot of attention was put to that issue.  So this code style conversation was put aside once more.  It was brought back up in September 2014 when we introduced the Rubocop gem to our code  base to deal with very basic code style rules such as white spaces.  If you don't yet know Rubocop, it's a Ruby gem.  It's open source, and it analyzes your code statically and serves as a linter to follow  many of the code style rules that were made by the community for the Ruby community.  The intention was great.  But we already had a very, wait, one second.  My thing doesn't work.  I don't know how to make it work.  It was a cute meme of this thing blowing up.  We had already a big code base.  It was 10 years old that had never been linted, so we had many violations.  Obviously, this exploded Rubocop because there was just too many things that needed to be  fixed at the same time.  So many of those cops were disabled, and we couldn't possibly fix all of the violations  then.  Now it will work?  No.  It doesn't want to work.  So instead of running Rubocop through the entire code base, we needed to find another solution  so that we could stop the bleeding temporarily.  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 police show jam came to life.  Police show, or if you're a Brazilian, you will call it policial because that's what  the name is supposed to be.  It's a wrapper built around Rubocop that only runs on PRs, so no commits and no, not the  master branch.  We need to accuse this code-style violations in code that was added or changed.  It was inspired in the Houndtool made by Thoughtbot, but it was actually adapted to what Shopify's  needs and constraints were back then.  Policial used, it listened to events using GitHub webhooks, and whenever a PR was created  by any one of our checkout team, it would comment on the PR with any code-style violations  that they had introduced.  The team was not very impressed.  The tool was very noisy, and it added a lot of comments in the PR, which is by far the  worst common complaint that I've heard while I was asking people about that.  Oh, it actually does work.  Despite the noise, they continued using it, but that meant that we were in for some much-needed  improvements to the tools that we were using.  October 2015 was quite an eventful month, where two new big changes came around.  The first one, it was when we were looking for other solutions, we thought about using  the commit status API in GitHub instead of commenting all over the PR to convey code-style  violations.  Much like a CI status, the police status changes depending on whether you found any violations  on the PR or not.  If no violations were found, it would be green, so that's great.  You could always merge if your reviews said so.  If you did find some violations, however, it would turn red, and you could click in the  details button on the side so that you could look at what those violations were.  This is what our Polatio tool looks like.  When you click the detail button, it would take you to this page.  You can see all of your style violations, the lines where they occur, and what cops and  violations you are infringing.  Back then, this was an opt-in feature that developers could choose whether they signed  up for it and then had their PRs checked by this tool.  Even though the tool wasn't extremely perfect, we saw a great adoption rate.  By the beginning of 2016, over 50% of our devs had already opted in.  The second thing that came around at that time was our Shopify Ruby style guide.  It came around together with the app so our developers could have a better idea of what  the code violations were, the style violations were, and how to fix them.  What was it that we were looking for?  This is our style guide today.  It was meant to set expectations for our Ruby developers, not only in terms of code style,  but also how they're encouraged to take ownership of their own code and decide for themselves  where exceptions to those rules can be reasonable and where they are allowed to do it.  But more important than those expectations, the guide offers plenty of examples of what  matches our code style and what doesn't.  It removes any guessing from when you're developing whether you are writing something correctly  or not.  A few months later, in March 2016, we sent out a survey to all our developers at Shopify  to find out the good, the bad, and the ugly about using PoliShow and our new style guide  that we had implemented so far.  Some of the questions that we asked were, do you believe PoliShow has a positive role  in our pull requests?  The majority of our developers said yes.  They really liked what the tool was doing.  They believed in what it was trying to achieve.  Those are some positive feedback that we got.  It said that it was much easier to onboard new people.  Now the code was more consistent.  It said that it allowed code reviews to focus on the code itself, whether it was there were  white spaces in the wrong places.  It also mentioned that it made code more readable.  Some negative feedback revolved around how PRs took longer to get done.  Devs would make a PR and then they would push their code to GitHub and then they would have  to fix all of the violations.  Then they would have to get feedback from their reviews and implement those as well.  So they were not super happy about that because it increased a lot their work.  The other comment that we got is that sometimes those changes made file less consistent so  it reduced readability.  I'll show you an example of one of those cases.  Here if we go back to the hash rocket example, line two is old code.  Line three is something that got newly added.  We have in the same file very close to each other.  We have something that is in our old code style and now something that is in new code  style.  That makes it very confusing.  Especially for new developers who are starting to learn Rails and Ruby.  Then we get questions of is there a performance difference among those two?  Can I use both?  Do both work?  So it wasn't ideal.  The next question that we asked was did you learn something valuable with Poly Show?  And again, most people said yes.  Some of the things that they learned is syntax improvements, things they didn't know, style  best practices in the Ruby community.  And it was easier for them to write code coming from non-Ruby backgrounds.  Some poor feedback that we got, some negative feedback that we got was that Ruby is not  new for me so I know all this stuff already and I can make those decisions for myself.  I don't need this tool making me decide, helping me decide this.  It also doesn't provide much insight into why the rules exist and how they improve our  code base.  Next question was does Poly Show make your life easier?  Again, most people said yes and because it was making the code base more predictable  so they could better understand it and it would help developers not have to worry about  whether putting the white space in this line or in that line because you would tell them  when it was wrong.  Some people who didn't like it or said there were things they could improve said they usually  had to spend more time in their code so they could fix those things.  The rules of violations were not always super clear and it was annoying that different projects  didn't use Poly Show and as such they would not have the same code style.  So one of the cool feedback that we got is that we should show all the projects in Shopify  and we knew at this point that these were not just opinions.  They were not, she said kind of opinions.  They were things that real developers who really use the tools were saying so that we  can in one way see what worked and in another side we could see what didn't work so what  could we do to improve those tools.  With such a great response we decided let's Poly Show all the PRs then.  We went ahead, we activated Poly Show for all the requests that were being opened in the  Shopify code base.  Other apps could also, we also allowed other apps to sign up for the service and they could  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, Poly Show was set up so that it wouldn't fail when it wouldn't fail  a build when there were code violations to a PR.  This isn't great because in one way people could just merge PRs even though there were  violations but in another way we could see as a positive thing because if there were  any emergencies that happened that we needed to merge PRs very fast we could choose to  merge them even though there were violations.  Our tools started to improve a little bit so people were getting acquainted with Poly  Show and RuboCop so we took the effort some steps further.  First we included RuboCop to run in our CI so it was only in one component so in most  of our checkout files RuboCop we ran through it, fixed all the violations and then from  then on your build would actually fail if you added new violations.  The second thing is that we decided to use tools, the tools that we had to improve other  parts of our development.  Back then our Rails team was upgrading Rails to Rails 5.0 and one of the issues they were  having is that people were adding, as we deleted some deprecations, new ones that were not  exactly related to cold style so we added some custom cops to prevent people to add  those new deprecations.  Here you can see an example of one of them.  This one checks if behaviors were being added directly to framework classes but instead  they should be actually using the onload hook provided by the Rails framework.  This cop was auto correctable so it was super useful.  You could just run the RuboCop dash A and it would fix all of those for you.  With its success we saw in the checkout team we were ready to roll it out to the entire  code base running all of RuboCop through everything getting rid of all the violations that we  currently had.  We had the perfect plan.  It was amazing.  Step one, we would run all the auto correctable cops through the entire code base.  We did it.  It was awesome.  Step two, we would decide on what was the best time to merge those PRs.  So we were looking for a development downtime to minimize conflict between our PRs and whatever  people were doing at the time.  So we chose the weekend because it's a time when not many people are working and there's  not much going on.  Step number three, we would send out an email at the beginning of the week on Monday to  all of the developers to let them know this was happening, to get everybody excited but  mostly to get people to prepare and if they had open pull requests that they would try  their best to merge them by Friday so that on Monday they didn't have a lot of conflicts  on their own PRs and that would have not been so great.  So we did all that except for the email.  Next weekend came about and we were like, yes, let's merge them all and have a LinkedIn  code base.  Okay, just kidding.  We did not.  Why not?  It was the perfect plan.  Why didn't we go and follow through with it?  We came across some very important blockers.  So it would break our Git history.  What does that mean?  Blames would be harder to do now because the code style fix commits would now pollute our  history by adding things that were not related to the functionality that the code was trying  to implement.  And the Shopify, we have a very important value that talks about ownership.  That means that wherever code that you touch in our code base, you want it now.  And back then, one person was running through all of the Ruby cops and a person was going  to merge them.  So now they would be responsible for most of our Shopify code base and then people get  pinged on Slack about all of the changes and changes that they might not exactly know that  were completely unrelated to their jobs.  The last thing, we came to a very real understanding of what a week at Shopify really is.  Back then, we were deploying about 30 times a day.  So a week's time was enough for a massive amount of code to have changed and been newly  added.  So the PRs that we had prepared the week before were actually all outdated and conflicting  with the new code that we had added during this week.  We were really not ready for this.  So we went ahead and closed up all those PRs and the issue that we were using to track  those.  And because our problems were all still there, so we were really not ready for this.  Policia was working.  The code was incrementally becoming more consistent, but by no means were the tools that we were  using making developers' lives easier.  Some of the issues were, like in the feedback forms that we have received, some of those  things came up.  So controversial rules.  We realized that we were enforcing rules that were either unclear or that didn't seem beneficial  for our code base.  All cops from Rubikop were enabled by default, and we were just removing the ones that we  actually didn't want.  Rails changed something in June last year to actually not do that and do the opposite  thing, to minimize our frustration among our devs, we instead follow Rails steps, and  instead of enabling all cops by default and removing the ones we didn't want, we would  start with zero and only add the ones that we actually wanted, because that's more Ruby.  Ruby-like, I would say.  This new strategy actually allowed us to discuss whether we should use, we should add a new  cop when it surfaced, and to stop discussing things that were not exactly useful for our  code base.  And so getting those cops out.  In January this year, GitHub added an update feature that allows you to easily look at  the history of a single line, and that came to be a very important feature that led us  to our next try.  So if you don't know this feature, that's what it does.  You can click on the little button and it will allow you to see, to go back on other  commits.  So we, a few months had passed, and a new strategy for cops worked quite well.  Our devs were more receptive today, the idea of having a code style guide, and they were  confident that while the journey wasn't great so far, we were aiming to improve it so that  life would be easier.  The journey wasn't all rainbows and butterflies, we appreciate that.  But we were really getting to a better place.  So we decided we were ready to try it again and use RuboCops out of correct feature to  fix all of our, of the violations to our entire code base.  But this time we were using a bot, so nobody would get bothered in Slack anymore.  This is what our bot looks like when it was merging those.  And for the violations that we were already correcting, like you saw in the screenshot  before, we were running the, we added the out of correct feature to our internal development  tool so that people instead of pushing PRs and waiting for those, for it to accuse for  new violations, they could actually use our internal tool to fix those before they even  push their code.  Also, we added, as soon as we merged those PRs with fixing all of those violations,  we now added those cops to our CI.  So our test suite would fail whenever new violations were added.  This is the issue that we were using to keep track of all the cops that we had already  used to link our code base.  But of course, not everything is perfect, right?  So we had a massive issue.  One of the biggest issues that we found was when we were adding those cops to our test  suite, linking the entire code base was taking a lot more time than our slowest test.  That wasn't great.  Developers were not going to be happy about this.  We knew it already.  So we tried to find several ways to fix that, and temporarily what we found is that we had  some giant files, migration files, and so we just excluded those from the files to be  linked in, and that helped a lot the time that it would take to link the entire code  base.  It's by no means a long-term solution, though.  In April 2007, this month, we had great responses from that, so some of our components in our  code base, like checkout, customers and gift cards, had been completely linted and are  now being fully checked at build time.  So I want to send some love here to our teams who actually took on and pushed through the  hard times and used those tools and gave us very impressive feedback on how can we improve  them and what they didn't like and what they did like.  So we're going to talk quickly about where we are today and where we need to get to.  So today, like I mentioned, we have some cops that will run with Polissio and some cops  that are running at build time.  So we have two RuboCop EML files today, so that's not ideal, and in the future, we want  to go back to having just one.  So fixing all the violations that we have today, moving those cops into our test suit  and then having them run only once.  Something else that developers were really not happy about is that pushing their code,  they had to wait for the whole thing to run so that they would see how many violations  there were and so on.  We want to have AutoCorrect enabled by default.  So instead of failing your build, it would actually, whenever you push your PR, it will  run through your entire code base.  It will make a commit that fixes all of your violations into your open PR and then it would  run through the test suit.  So then we wouldn't have to worry about it at all.  Like I mentioned some minutes ago, we had issues with our Ruby linters taking too long.  So fixing this isn't going to be easy.  We have a solution right now, but it's not ideal, and we'll have to improve, so we'll  have to use caching to be able to fix the problem in the long term, especially as we  add more code to our code base.  A lot of the issues we encounter, I knew, and while we tackle them, it's also important  for us to push them upstream so that the community can also benefit from those improved tools.  And one important thing is that we want to continue improving our style guide because  it gives more context and clarity behind why a cop exists, why, how does it benefit our  code base and readability and all of other positive things that come out of having one  very consistent file.  So let's talk about some final takeaways that we took from this 13-year journey.  The most important thing, and this is the thing that you want to take back with you,  is that whenever you're thinking about this effort, you need to have a place for discussion.  Developers are very opinionated a lot of the times, and they would like to sometimes discuss  whether this is good or this is not.  It might not work for you, but it might not work for a very large number of people, so  you want to have that place where they can actually chat about it and bring new ideas,  which brings me to my next point.  Pick styles that work for your code base.  In many places, RuboCop is not very opinionated about what should your style be like.  You can pick one of the many that they offer, and you can talk to other people in your team  to decide what's the best one for you.  So here is one example of something that is good and good, so you can just pick one,  but talk to the people that work with you and decide what's the best one for you.  Allow for rules to be challenged.  It's more or less the same thing.  Just because it's there, 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.  Your tools have to be somewhat prepared, and your developers need to be at least partially  on board for this change to be successful.  But don't give up at hard times.  It's not going to be an easy journey.  The time is never going to be absolutely perfect.  Choose a time when it's good enough, but don't give up.  Talk to others.  Learn more about your tools so that you know what can and can't be done.  Tackle one thing at a time.  But the most important thing is have fun through the process.  I've learned a lot through working, trying to tackle this problem at Shopify.  I met a lot of awesome people, and so can you.  Thank you so much.  Thanks to the developers for making this opportunity to speak to me.  So it's coming.
Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_0_335.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_336_1883.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_1884_3407.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_3408_3935.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_3936_5579.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_5580_5915.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_5916_6035.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_6036_6251.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_6252_6335.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_6336_7331.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_7332_12851.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_12852_14159.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_14160_17447.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_17448_17891.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_17892_18179.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_18180_18239.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_18240_21731.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_21732_22607.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_22608_22655.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_22656_23663.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_23664_27635.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_27636_27983.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_27984_28907.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_28908_31439.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_31440_31955.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_31956_32051.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_32052_32567.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_32568_32627.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_32628_35879.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_35880_36455.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_36456_38603.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_38604_38915.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_38916_40655.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_40656_40787.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_40788_41279.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_41280_41555.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_41556_41783.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_41784_43103.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_43104_49967.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_49968_50147.png Keeping_Code_Style_Sanity_in_a_10-year-old_Codebase_2_50148_50173.png