4 ms·

Here is the method I used in a similar situation. Start from an extremely loyal linter config where you get 0 errors and 0 warnings with your existing code (tha

by kozak 9y ago

Here is the method I used in a similar situation. Start from an extremely loyal linter config where you get 0 errors and 0 warnings with your existing code (that might mean an empty config, but that's OK). Then add rules one by one, fixing the codebase completely for each rule that you add, so that you always have 0 warnings and 0 errors with each commit. Yes, this is significant amount of work, but at least the work is broken down into chunks, and each chunk has a clear definition of done (can go straight into an agile backlog, etc). By the time you have a sensible linter config, you have a much better codebase as well.

aizatto 9y ago
Ditto, this is the same thing I did for a 16,000+ line app. I used eslint ( https://eslint.org https://eslint.org ) and prettier ( https://prettier.io https://prettier.io ), and using airbnb's JavaScript Style Guide as a reference point ( https://github.com/airbnb/javascript https://github.com/airbnb/javascript ), I would enable rules one by one. Then slowly rule by rule (commit by commit) I would clean up the code base.
- _pdp_ 9y ago
  Our modules currently are at 96398 lines of code (I am not counting the apps). It is still maintainable and perfectly understandable. Everything is split into module though and then every module has its own tests and its own examples. We lack proper documentation. It is not a problem yet but it could turn out to be in the future.
wschoi 9y ago
I feel the same that it can be overwhelming to apply linters to legacy code base. While developing DeepScan, we tried to aggresively filter-out low impact issues and common coding patterns that are not actual problems.
robinwassen 9y ago
Segmentation is the way to go, your approach is risky since you need to change a lot of different parts of the project. We segmented by creating a legacy folder with the old standard and a folder for the new code that has strict linting rules. If you created something new or did substantial changes to something existing it should go into the new folder. That way the code quality improvement became a natural part of our work process, and we did not have to touch parts just to improve code quality. For that serves quite little value by itself.
- badshar 9y ago
  What do you do when you have an old school application where JavaScript is written in non-modern style? For example, you have a.js and b.js. You define a function "hello" in a.js and use it in b.js. In the project, the developer assumes that the two files will be included in correct order within HTML. Linters fail here because they think b.js uses an undefined function, when it's actually defined. Unfortunately, since most JS linters only validate one file at a time, they are pretty much useless here.
  - dugword 9y ago
    You add 'hello' in the linter's config as a pre-defined global, until you get around to fixing that. I would not want my linter to load multiple files, since that pattern of polluting the global namespace is really not maintainable. The fix can be fairly simple if you wrap your entire b.js file in a function that takes an argument of 'hello' which you invoke from a.js. If that doesn't work because you have too many global declarations you should probably just dump all the code into one giant file. That might seem terrible and unmaintainable, but it is exactly what you already have. Once the mess is all in one spot you can use a linter to start improving the quality, and begin breaking of the code into actual modules.
    - badshar 9y ago
      This doesn't work on large projects with multiple people involved. Can't go through hundreds of JS files and manually add every function to the linter's config.
    - mderazon 9y ago
      > just dump all the code into one giant file Sounds too scary. I am facing the same problem of too many global declarations all thrown into a global soup during build time when all js files are concatenated. I am looking for a safe and methodological method. By safe I mean a method that a robot can understand. If you leave room for thinking then people start making mistakes. It's hard enough to get the green light for starting such project. If you start breaking stuff as well, you're going to burn the credit for refactor real quick
SaltyBackendGuy 9y ago
This is exactly where my code base was about 11 months ago. We just got a new hire and he commented on how nice the code was (Should have saw it 11 months ago haha). It all started with linter config, piece by piece it started looking better and more maintainable (also refactoring to es7 features helped). You just have to do it, one small part/ticket at a time.
dugword 9y ago
I have taken a similar approach where I start with having 0 lint issues, and add a git hook or CI step to only allow merges with 0 lint issues. Except instead of modifying my config over time, I add a /* eslint ignore */ to the top of every existing file. Every new file will be coded to the new standard, and as I open old files to tweak them I take the opportunity to clean them up.

by kozak 9y ago

aizatto 9y ago

Ditto, this is the same thing I did for a 16,000+ line app. I used eslint ( https://eslint.org https://eslint.org ) and prettier ( https://prettier.io https://prettier.io ), and using airbnb's JavaScript Style Guide as a reference point ( https://github.com/airbnb/javascript https://github.com/airbnb/javascript ), I would enable rules one by one. Then slowly rule by rule (commit by commit) I would clean up the code base.

_pdp_ 9y ago
Our modules currently are at 96398 lines of code (I am not counting the apps). It is still maintainable and perfectly understandable. Everything is split into module though and then every module has its own tests and its own examples. We lack proper documentation. It is not a problem yet but it could turn out to be in the future.

wschoi 9y ago

I feel the same that it can be overwhelming to apply linters to legacy code base. While developing DeepScan, we tried to aggresively filter-out low impact issues and common coding patterns that are not actual problems.

robinwassen 9y ago

Segmentation is the way to go, your approach is risky since you need to change a lot of different parts of the project. We segmented by creating a legacy folder with the old standard and a folder for the new code that has strict linting rules. If you created something new or did substantial changes to something existing it should go into the new folder. That way the code quality improvement became a natural part of our work process, and we did not have to touch parts just to improve code quality. For that serves quite little value by itself.

badshar 9y ago
What do you do when you have an old school application where JavaScript is written in non-modern style? For example, you have a.js and b.js. You define a function "hello" in a.js and use it in b.js. In the project, the developer assumes that the two files will be included in correct order within HTML. Linters fail here because they think b.js uses an undefined function, when it's actually defined. Unfortunately, since most JS linters only validate one file at a time, they are pretty much useless here.
- dugword 9y ago
  You add 'hello' in the linter's config as a pre-defined global, until you get around to fixing that. I would not want my linter to load multiple files, since that pattern of polluting the global namespace is really not maintainable. The fix can be fairly simple if you wrap your entire b.js file in a function that takes an argument of 'hello' which you invoke from a.js. If that doesn't work because you have too many global declarations you should probably just dump all the code into one giant file. That might seem terrible and unmaintainable, but it is exactly what you already have. Once the mess is all in one spot you can use a linter to start improving the quality, and begin breaking of the code into actual modules.
  - badshar 9y ago
    This doesn't work on large projects with multiple people involved. Can't go through hundreds of JS files and manually add every function to the linter's config.
  - mderazon 9y ago
    > just dump all the code into one giant file Sounds too scary. I am facing the same problem of too many global declarations all thrown into a global soup during build time when all js files are concatenated. I am looking for a safe and methodological method. By safe I mean a method that a robot can understand. If you leave room for thinking then people start making mistakes. It's hard enough to get the green light for starting such project. If you start breaking stuff as well, you're going to burn the credit for refactor real quick

SaltyBackendGuy 9y ago

This is exactly where my code base was about 11 months ago. We just got a new hire and he commented on how nice the code was (Should have saw it 11 months ago haha). It all started with linter config, piece by piece it started looking better and more maintainable (also refactoring to es7 features helped). You just have to do it, one small part/ticket at a time.

dugword 9y ago

I have taken a similar approach where I start with having 0 lint issues, and add a git hook or CI step to only allow merges with 0 lint issues. Except instead of modifying my config over time, I add a /* eslint ignore */ to the top of every existing file. Every new file will be coded to the new standard, and as I open old files to tweak them I take the opportunity to clean them up.