CodeWithAsheville / court-notifications

GNU General Public License v3.0
11 stars 10 forks source link

refactor: remove unnecessary code #164

Closed Dun-sin closed 2 years ago

Dun-sin commented 2 years ago

name: "refactor: remove unnecessary code" about: "I'm ready to check in some code to fix an issue!" title: "2022-08-07: Remove unnecessary code" assignee: Dun-sin

Closes #162

Please check if the PR fulfills these requirements

What changes are you introducing?

Screenshots or Video:

ejaxon commented 2 years ago

Hey, @Dun-sin, thanks for submitting this. I'm traveling at the moment and unable to look, so it may be a few days before I can review. Just wanted to give you a heads up. -Eric

ejaxon commented 2 years ago

Hi @Dun-sin, could you reformat to match the indents in the code base (2 spaces)? The addition of large indents makes it very hard to see exactly what has changed and creates a mis-match with the rest of the code.

Thanks,

Eric

Dun-sin commented 2 years ago

Sure will do that, but to make it pretty easy, so anyone doesn't alter style settings like this, it will be better to add a prettier file, maybe make an issue about it, up to you.

It's very hard to style it as you want right now but if prettier is added, it will fix everything properly

Dun-sin commented 2 years ago

I really tried, but it's hard, the best way to fix this in every code is to add a prettier file

ejaxon commented 2 years ago

Hi @Dun-sin, I'm reopening because I want to make the corrections, though I probably won't just accept the request given the formatting issues.

I'm hoping to finally get to all this this weekend - family issues have taken up all my spare time over the last few weeks.

I think my preference is to start with ESLint rather than Prettier, but I agree with you that we should have an automated way fo at least flagging formatting errors. I will start working on that as soon as I can, unless you have any interest in doing so.

Dun-sin commented 2 years ago

Hi @Dun-sin, I'm reopening because I want to make the corrections, though I probably won't just accept the request given the formatting issues.

I'm hoping to finally get to all this this weekend - family issues have taken up all my spare time over the last few weeks.

I think my preference is to start with ESLint rather than Prettier, but I agree with you that we should have an automated way fo at least flagging formatting errors. I will start working on that as soon as I can, unless you have any interest in doing so.

You can mix ESLint and prettier, and yes, I can work on this. But once you run it, it's going to change every file to the right formatting

ejaxon commented 2 years ago

Ok, I'm a big fan of doing things one step at a time. I'm going to make the changes from your pull request since they are all going to be caught by a linter anyway. Then I'll set up ESLint with the AirBnB configuration and get the code clean according to that. Once that's done, we can add prettier (I assume there aren't huge formatting differences between prettier and AirBnB?).

Dun-sin commented 2 years ago

Ok, I'm a big fan of doing things one step at a time. I'm going to make the changes from your pull request since they are all going to be caught by a linter anyway. Then I'll set up ESLint with the AirBnB configuration and get the code clean according to that. Once that's done, we can add prettier (I assume there aren't huge formatting differences between prettier and AirBnB?).

Prettier isn't exactly strict for styling like eslint, eslint just makes sure coding guidelines are followed I guess

ejaxon commented 2 years ago

@Dun-sin - I'm going to go ahead and close this. I made the substantive changes you had. I have also added the use of eslint using the AirBnB configuration on the server code and cleaned up all the linting errors there. I'm merging into development, but want to do a fair amount of testing before I push anything on to staging or production.

Once that's all through, we can take a look at maybe adding prettier as a next step.