5 ms·

Yeah, there's a process. It's something that I've done a bunch of times for a bunch of clients. There's so much low-hanging fruit there that's so easy to fix _

Yeah, there's a process. It's something that I've done a bunch of times for a bunch of clients. There's so much low-hanging fruit there that's so easy to fix _right now_. No version control? Good news! `git init` is free! PHPCS/PHP-CS-fixer can normalise a lot, and is generally pretty safe (especially when you have git now). Yeah, it's overwhelming, but OP said that the software is already making millions - you don't wanna fuck with that. I've done it, I've written about it, I've given conference talks about it. The real bonus for OP is that the team is small, so there's only a few people to fight over it. It's pretty easy to show how things will be better, but remember that the team are going to resist deleting code not because that they're unaware that it's bad, but because they are afraid to jeporadise whatever stability that they've found.

ido 4y ago
And also starting by fixing the js/css/html front end is likely the safest, as it wont corrupt any customer data & it will be visible when something breaks. That can probably be the next best candidate to do a major overhaul. I'd also hope that a $20M/year project can afford to hire someone senior in addition to these 3 juniors?
- ethbr0 4y ago
  > hope that a $20M/year project can afford to hire someone senior Never underestimate the ability of management to look a gift horse in the mouth while shooting it in the foot.
- denimnerd42 4y ago
  why would someone senior even want to join this team? Especially someone senior enough to fix this. The productivity is horrible and there's no kudos for fixing something that's lived for 12 years like this.
  - JoeAltmaier 4y ago
    I was added to a team because, to quote the VP, "they're good but they need some adult supervision" Mixing skill levels in a team is healthy.
  - lolinder 4y ago
    Theoretically a company that's making $20m/year on this can afford to make it worth someone's while to come in and fix it. The problem isn't finding someone who will do it, it's that the company assumes they can continue to get by indefinitely on paying too little.
  - marcosdumay 4y ago
    > why would someone senior even want to join this team? Well, money. Why would someone even want to join any team?
  - majikandy 4y ago
    For the love of refactoring.
nkozyra 4y ago
git init seems like job #1 because at least then you can delete every commented out line and start a little cleaner.
- gloosx 4y ago
  to loose all the comments? :D that would make it even harder to read
  - lolinder 4y ago
    In a project without version control (or one that doesn't trust it enough) there are always whole sub-programs made up of dead code. It's usually some combination of commented-out blocks and functions that are only called from within those commented-out blocks. Removing commented code (not real, descriptive comments) is the first step to eliminating all this dead code, and eliminating dead code buys a ton more flexibility in what you can change safely.
    - pdimitar 4y ago
      Fully agreed, I was tasked with using an old library and my first order of business was to make an analysis of dead code branches. The GIT commit removed 17 out of 80 files and about 10-11% of the code in some other files (that were not deleted) and the library works 100% the same -- confirmed by tests that I painstakingly added during the last weeks. Less code, less confusion.
jspash 4y ago
Personally, I would never run a linter of any kind on a full codebase that doesn't have tests. After having been bitten by all kinds of bugs over the years, I wouldn't suggest auto-linting any file that you aren't actively working on. It's rare that linting will actually make the code work better. Granted, it could catch some security bugs. But they can - and will - introduce new bugs. You just have to ask if it's worth the risk.
- dtgriscom 4y ago
  This. It's so tempting when a linter warns "This code is misleading; it would be clearer to do it this other way" to think "Easy fix: change it the way the linter suggests." But, make the change, and you may discover (hopefully before delivery) that the code functionality depends on the confusing behavior.

ido 4y ago

And also starting by fixing the js/css/html front end is likely the safest, as it wont corrupt any customer data & it will be visible when something breaks. That can probably be the next best candidate to do a major overhaul. I'd also hope that a $20M/year project can afford to hire someone senior in addition to these 3 juniors?

ethbr0 4y ago
> hope that a $20M/year project can afford to hire someone senior Never underestimate the ability of management to look a gift horse in the mouth while shooting it in the foot.
denimnerd42 4y ago
why would someone senior even want to join this team? Especially someone senior enough to fix this. The productivity is horrible and there's no kudos for fixing something that's lived for 12 years like this.
- JoeAltmaier 4y ago
  I was added to a team because, to quote the VP, "they're good but they need some adult supervision" Mixing skill levels in a team is healthy.
- lolinder 4y ago
  Theoretically a company that's making $20m/year on this can afford to make it worth someone's while to come in and fix it. The problem isn't finding someone who will do it, it's that the company assumes they can continue to get by indefinitely on paying too little.
- marcosdumay 4y ago
  > why would someone senior even want to join this team? Well, money. Why would someone even want to join any team?
- majikandy 4y ago
  For the love of refactoring.

nkozyra 4y ago

git init seems like job #1 because at least then you can delete every commented out line and start a little cleaner.

gloosx 4y ago
to loose all the comments? :D that would make it even harder to read
- lolinder 4y ago
  In a project without version control (or one that doesn't trust it enough) there are always whole sub-programs made up of dead code. It's usually some combination of commented-out blocks and functions that are only called from within those commented-out blocks. Removing commented code (not real, descriptive comments) is the first step to eliminating all this dead code, and eliminating dead code buys a ton more flexibility in what you can change safely.
  - pdimitar 4y ago
    Fully agreed, I was tasked with using an old library and my first order of business was to make an analysis of dead code branches. The GIT commit removed 17 out of 80 files and about 10-11% of the code in some other files (that were not deleted) and the library works 100% the same -- confirmed by tests that I painstakingly added during the last weeks. Less code, less confusion.

jspash 4y ago

Personally, I would never run a linter of any kind on a full codebase that doesn't have tests. After having been bitten by all kinds of bugs over the years, I wouldn't suggest auto-linting any file that you aren't actively working on. It's rare that linting will actually make the code work better. Granted, it could catch some security bugs. But they can - and will - introduce new bugs. You just have to ask if it's worth the risk.

dtgriscom 4y ago
This. It's so tempting when a linter warns "This code is misleading; it would be clearer to do it this other way" to think "Easy fix: change it the way the linter suggests." But, make the change, and you may discover (hopefully before delivery) that the code functionality depends on the confusing behavior.