7 ms·

Self-Protecting Projects

44 points by amihaiemil 7y ago

cerberusss 7y ago
Author, I'm a sucker for spelling. (edit: removed). Good blog post. When I was programming Java, I also used Checkstyle. It's a long-running and reliable project.
- amihaiemil 7y ago
  Thanks for the comment. I am a sucker for spelling indeed. I usually spend about 1h after publishing each post, to fix typos (I never notice them until it's published, no matter how hard I try).
  - blowski 7y ago
    I find when I write, the free http://hemingwayapp.com http://hemingwayapp.com website is really useful for fixing basic spelling, punctuation and grammar errors.
    - amihaiemil 7y ago
      Thank you. I know about these apps, especially Grammarly. I just don't like the idea that all I write goes to a server somewhere :) I also noticed Grammarly sticking its nose in places it shouldn't have. Like private repos on Github.
      - Sean1708 7y ago
        Why not use a local spell checker instead?
        amihaiemil 7y ago
        The ones that IDE provide? Those are rather primitive, don't you think?
        Sean1708 7y ago
        They would still catch all the incorrect spellings, they just wouldn't catch the incorrect grammar. The grammar of the post is fine, though, so that wouldn't matter too much. You could even just copy and paste it into a word processor, if you also want to catch grammar issues.
  - cerberusss 7y ago
    Well, it should never get in the way of a good blog post! Kudos to you.
blowski 7y ago
I find it strange that senior developers would laugh at the idea of using quality control gates in a CI/CD pipeline. I know a lot of people don't bother with these things, but most seniors I've worked with would at least aspire to (or say they aspire to) using such techniques. But I guess that's the difference between a senior who knows lots of syntax, and a senior that that knows how to deliver quality.
- cryptica 7y ago
  I've worked for many companies that had fully automated CI/CD pipelines. If you have a good team which cares about the project, then you don't really need them IMO. The reality of most big corporations is that nobody actually cares about the company. Executives can't trust their engineers to run the tests manually and deliver something that works. If you have a good hiring and promotion process and a fair compensation structure (which none of the big corporations have), then you can trust your employees and so you don't need to micromanage them with automated CI/CD pipelines. In addition, if people actually care, you get much higher quality, more succinct code. No amount of automated checking can lead to improved code quality. It only ensures that the absolute minimum operational requirements are met at any given time.
  - blauditore 7y ago
    > If you have a good team which cares about the project, then you don't really need them IMO. I strongly disagree. It's not about caring or not, it's about making mistakes. A human can easily forget to run some test, or generally one step out of many in a complicated procedure, especially if they're new to the project. A reviewer can miss a mis-formed identifier. The more is that automated, the fewer manual steps need to be remembered, and the more safety the project gets. Automated tools like CI/CD should not be considered supervisors to humans, but helpers and safety nets. That being said, there are certainly cases where teams don't set such tools up because they come with an implementation cost. And depending on project complexity and their use case in general, it may be more economical to do things manually.
    - 7y ago
clktmr 7y ago
Wherever I worked, these kind of checks did always more harm than good. You force a bunch of tools on your developers which they hate, only to have your whole codebase littered with "disable linter X" comments. In a good review process the developers can negotiate the conventions by themselves, which leads to acceptance among the team.
- amihaiemil 7y ago
  That's just a matter of methodology and/or discipline. Depends on the team: if the team is democratic, then they should reach an agreement beforehand and respect it. If it's a dictatorship, then just punish them for not respecting the rules. If the devs just agree to break the rules and disable them, they should be punished somehow. "Punished" is a big word maybe, but that's the idea :)
- calpaterson 7y ago
  Well if the team doesn't like the linter tool they should be able to change it. That's a political problem and nothing to do with linters. My experience is that linters (that people like) saves you from having to constantly discuss formatting, minor problems at the PR stage which is otherwise a huge time sink.
- amihaiemil 7y ago
  In one of the teams I best worked with, payment was done per closed task. There were no salaries. Just bounties per closed tasks. If you didn't respect the Quality Gates, your branch wouldn't be merged => you wouldn't be paid. It was one of the best experiences in my work life so far. Clear rules, no chaos, no meetings, no negotiation. Just tasks and money :)
  - tomashubelbauer 7y ago
    I can think of a few ways of how this could backfire / be gamed or abused, but all in all, seeing how everything else is just as susceptible to abuse and gaming, it still sounds awesome to me.
overlordalex 7y ago
Nice article - one nitpick is that I absolutely do /not/ make the checks mandatory as part of the build process. Rather there are some "opinionated" checks that are run when code is pushed for review (which can be skipped), and then strong checks run as the first step in the CI/CD pipelines. This means that the tools stay out of the way when developing and running locally, but still enforce standards where required. This leads to fewer developers disabling or @ignore-ing rules to test things locally and then forgetting to remove them. This article also reminds me of a talk I saw from Neal Ford about architectural fitness functions: the idea being that if there is an architectural pattern that should be followed then the best place to put it is in an automated step as part of your CI/CD. I thought it was interesting to take the concept normally limited to linters and apply it to a more abstract principle
- eru 7y ago
  I agree. Curiously, the authors of the Go language disagree.
  - SAI_Peregrinus 7y ago
    I think it depends on the check. Some checks are better enforced strictly in an automated fashion. Style rules are a good example: they make no performance difference, lead to bikeshedding, and are pretty much arbitrary. Other checks should be made automatically but not strictly enforced. EG you might want to require all C casts to have a comment describing why it's safe. That can be a good linter warning, and linters tend to have codes that can be added in comments to suppress the warning. So the linter warning can make code review easier, but shouldn't necessarily fail the build.
    - eru 7y ago
      Go fails the build, when you have an unused variable.
peterkelly 7y ago
Great, so now I have 100,000 duplicate bug reports to sort through because I forgot a null check and the logger automatically created an issue each time that error occurred (with different timestamps, causing duplicate detection to fail).
- amihaiemil 7y ago
  The duplication Issue can be solved in many ways. First one that comes to mind is, the Issue Tracker should have a limit on the number of tickets a certain User can open. If the Issue Tracker doesn't have that, then the company can have a firewall rule to only allow a certain number of calls per day to the API. I'm sure there are dozens of other possible scenarios :)
  - fapjacks 7y ago
    So not only do you fill the issue tracker with ten thousand duplicate issues, but now you are blind to issues numbered #10000 or higher.
kkapelon 7y ago
>Writing about this now, I got an idea: we should have some sort of plugin for Log4J or slf4J: a plugin that would automatically open tickets on Github or other trackers, when the .error(...) method is called Great idea in theory. It will not fly in practice. If you tell to any enterprise company that your (or their) software is automatically doing this behind the scenes, the project will be shot down. I mean, getting crash metrics is one thing, but opening automated issues from enterprise software is a completely different manner.
- TYPE_FASTER 7y ago
  I like the approach Visual Studio App Center (and probably others) takes of aggregating crash reports so you can easily figure out which issues are impacting the most users and prioritize.
alessioalex 7y ago
Weird. Everybody that I know seems to be using ESLint in some way or another, which has rules for whatever you can think of.
fapjacks 7y ago
This sounds nice on the surface, good intent. But it smacks of inexperience and is the same mechanism that ends up pushing the kinds of dumb conventions that insist on golang's Trailing Comma. What is the expected outcome of your rule, and how do you expect additional, incremental bureaucracy to drive that outcome? Any time you push rules, you must expect that people will default to simply obeying the rule without questioning why the rule exists in the first place. This is just a natural cost-minimizing function of human beings. As a former console operator of a decades-old mainframe, let me warn you that this is how informal "operator manuals" end up spanning many three-ring binders filled with nonsensical, sometimes contradictory trivia, and how you end up with people going through the motions like some kind of ritual whose origins are lost in the mists of time. If you're going to do something like this, I suggest that these kinds of "code quality gateways" aren't as absolute as OP recommends. Be smart about it: You could probably measure how "sloppy" a developer is (relative to some mean of their own output and not compared to the team or really anyone else). Simply bringing it to a developer's attention (programmatically) will do far more for your project's code quality than trying to dumbly enforce some list of code conventions about whitespace or whatever. Also, this is a kind of tool that should absolutely be scoped strictly on the level of the individual developer, with no possibility of having externally-visible reports generated from it, and absolutely not exposed to management. You don't want to give management too many knobs to turn, especially knobs like this.

44 points by amihaiemil 7y ago

Author, I'm a sucker for spelling. (edit: removed). Good blog post. When I was programming Java, I also used Checkstyle. It's a long-running and reliable project.

amihaiemil 7y ago
Thanks for the comment. I am a sucker for spelling indeed. I usually spend about 1h after publishing each post, to fix typos (I never notice them until it's published, no matter how hard I try).
- blowski 7y ago
  I find when I write, the free http://hemingwayapp.com http://hemingwayapp.com website is really useful for fixing basic spelling, punctuation and grammar errors.
  - amihaiemil 7y ago
    Thank you. I know about these apps, especially Grammarly. I just don't like the idea that all I write goes to a server somewhere :) I also noticed Grammarly sticking its nose in places it shouldn't have. Like private repos on Github.
    - Sean1708 7y ago
      Why not use a local spell checker instead?
      - amihaiemil 7y ago
        The ones that IDE provide? Those are rather primitive, don't you think?
        Sean1708 7y ago
        They would still catch all the incorrect spellings, they just wouldn't catch the incorrect grammar. The grammar of the post is fine, though, so that wouldn't matter too much. You could even just copy and paste it into a word processor, if you also want to catch grammar issues.
- cerberusss 7y ago
  Well, it should never get in the way of a good blog post! Kudos to you.

blowski 7y ago

I find it strange that senior developers would laugh at the idea of using quality control gates in a CI/CD pipeline. I know a lot of people don't bother with these things, but most seniors I've worked with would at least aspire to (or say they aspire to) using such techniques. But I guess that's the difference between a senior who knows lots of syntax, and a senior that that knows how to deliver quality.

cryptica 7y ago
I've worked for many companies that had fully automated CI/CD pipelines. If you have a good team which cares about the project, then you don't really need them IMO. The reality of most big corporations is that nobody actually cares about the company. Executives can't trust their engineers to run the tests manually and deliver something that works. If you have a good hiring and promotion process and a fair compensation structure (which none of the big corporations have), then you can trust your employees and so you don't need to micromanage them with automated CI/CD pipelines. In addition, if people actually care, you get much higher quality, more succinct code. No amount of automated checking can lead to improved code quality. It only ensures that the absolute minimum operational requirements are met at any given time.
- blauditore 7y ago
  > If you have a good team which cares about the project, then you don't really need them IMO. I strongly disagree. It's not about caring or not, it's about making mistakes. A human can easily forget to run some test, or generally one step out of many in a complicated procedure, especially if they're new to the project. A reviewer can miss a mis-formed identifier. The more is that automated, the fewer manual steps need to be remembered, and the more safety the project gets. Automated tools like CI/CD should not be considered supervisors to humans, but helpers and safety nets. That being said, there are certainly cases where teams don't set such tools up because they come with an implementation cost. And depending on project complexity and their use case in general, it may be more economical to do things manually.
  - 7y ago

clktmr 7y ago

Wherever I worked, these kind of checks did always more harm than good. You force a bunch of tools on your developers which they hate, only to have your whole codebase littered with "disable linter X" comments. In a good review process the developers can negotiate the conventions by themselves, which leads to acceptance among the team.

amihaiemil 7y ago
That's just a matter of methodology and/or discipline. Depends on the team: if the team is democratic, then they should reach an agreement beforehand and respect it. If it's a dictatorship, then just punish them for not respecting the rules. If the devs just agree to break the rules and disable them, they should be punished somehow. "Punished" is a big word maybe, but that's the idea :)
calpaterson 7y ago
Well if the team doesn't like the linter tool they should be able to change it. That's a political problem and nothing to do with linters. My experience is that linters (that people like) saves you from having to constantly discuss formatting, minor problems at the PR stage which is otherwise a huge time sink.
amihaiemil 7y ago
In one of the teams I best worked with, payment was done per closed task. There were no salaries. Just bounties per closed tasks. If you didn't respect the Quality Gates, your branch wouldn't be merged => you wouldn't be paid. It was one of the best experiences in my work life so far. Clear rules, no chaos, no meetings, no negotiation. Just tasks and money :)
- tomashubelbauer 7y ago
  I can think of a few ways of how this could backfire / be gamed or abused, but all in all, seeing how everything else is just as susceptible to abuse and gaming, it still sounds awesome to me.

overlordalex 7y ago

Nice article - one nitpick is that I absolutely do /not/ make the checks mandatory as part of the build process. Rather there are some "opinionated" checks that are run when code is pushed for review (which can be skipped), and then strong checks run as the first step in the CI/CD pipelines. This means that the tools stay out of the way when developing and running locally, but still enforce standards where required. This leads to fewer developers disabling or @ignore-ing rules to test things locally and then forgetting to remove them. This article also reminds me of a talk I saw from Neal Ford about architectural fitness functions: the idea being that if there is an architectural pattern that should be followed then the best place to put it is in an automated step as part of your CI/CD. I thought it was interesting to take the concept normally limited to linters and apply it to a more abstract principle

eru 7y ago
I agree. Curiously, the authors of the Go language disagree.
- SAI_Peregrinus 7y ago
  I think it depends on the check. Some checks are better enforced strictly in an automated fashion. Style rules are a good example: they make no performance difference, lead to bikeshedding, and are pretty much arbitrary. Other checks should be made automatically but not strictly enforced. EG you might want to require all C casts to have a comment describing why it's safe. That can be a good linter warning, and linters tend to have codes that can be added in comments to suppress the warning. So the linter warning can make code review easier, but shouldn't necessarily fail the build.
  - eru 7y ago
    Go fails the build, when you have an unused variable.

peterkelly 7y ago

Great, so now I have 100,000 duplicate bug reports to sort through because I forgot a null check and the logger automatically created an issue each time that error occurred (with different timestamps, causing duplicate detection to fail).

amihaiemil 7y ago
The duplication Issue can be solved in many ways. First one that comes to mind is, the Issue Tracker should have a limit on the number of tickets a certain User can open. If the Issue Tracker doesn't have that, then the company can have a firewall rule to only allow a certain number of calls per day to the API. I'm sure there are dozens of other possible scenarios :)
- fapjacks 7y ago
  So not only do you fill the issue tracker with ten thousand duplicate issues, but now you are blind to issues numbered #10000 or higher.

kkapelon 7y ago

>Writing about this now, I got an idea: we should have some sort of plugin for Log4J or slf4J: a plugin that would automatically open tickets on Github or other trackers, when the .error(...) method is called Great idea in theory. It will not fly in practice. If you tell to any enterprise company that your (or their) software is automatically doing this behind the scenes, the project will be shot down. I mean, getting crash metrics is one thing, but opening automated issues from enterprise software is a completely different manner.

TYPE_FASTER 7y ago
I like the approach Visual Studio App Center (and probably others) takes of aggregating crash reports so you can easily figure out which issues are impacting the most users and prioritize.

alessioalex 7y ago

Weird. Everybody that I know seems to be using ESLint in some way or another, which has rules for whatever you can think of.

fapjacks 7y ago

This sounds nice on the surface, good intent. But it smacks of inexperience and is the same mechanism that ends up pushing the kinds of dumb conventions that insist on golang's Trailing Comma. What is the expected outcome of your rule, and how do you expect additional, incremental bureaucracy to drive that outcome? Any time you push rules, you must expect that people will default to simply obeying the rule without questioning why the rule exists in the first place. This is just a natural cost-minimizing function of human beings. As a former console operator of a decades-old mainframe, let me warn you that this is how informal "operator manuals" end up spanning many three-ring binders filled with nonsensical, sometimes contradictory trivia, and how you end up with people going through the motions like some kind of ritual whose origins are lost in the mists of time. If you're going to do something like this, I suggest that these kinds of "code quality gateways" aren't as absolute as OP recommends. Be smart about it: You could probably measure how "sloppy" a developer is (relative to some mean of their own output and not compared to the team or really anyone else). Simply bringing it to a developer's attention (programmatically) will do far more for your project's code quality than trying to dumbly enforce some list of code conventions about whitespace or whatever. Also, this is a kind of tool that should absolutely be scoped strictly on the level of the individual developer, with no possibility of having externally-visible reports generated from it, and absolutely not exposed to management. You don't want to give management too many knobs to turn, especially knobs like this.