Hoist baseline eslint and prettier config for app

[left-on-read]

Hoist baseline eslint and prettier config for app

[alexdanilowicz]

1. You want to change the electron's app pretitier tabwidth from 4 to 2? Just in the name of consistency to match the server/web repors? The prettier / eslint currently in app is all hijacked from the electron boiler plate repo. We can do this, it will just cause the style github workflow to run every time EDIT: unless we just do a massive prettier write . commit.. seems like overkill IMO?

2. also — No, I don't want to turn on no-explicit-any because the originally boilerplate code uses any and I'm not sure what type it's suppose to be for a few of them (and clearly the authors didn't either). Same thing for prefer-default-export .... I don't see the value in turning in on now? @Teddarific

[alexdanilowicz]

also, I noticed you've been pushing directly to main. FYI the linting workflow only runs on PR builds https://github.com/Left-on-Read/leftonread/blob/main/.github/workflows/app-format.yml (looks like you haven't done any work on the app so more just FYI). I don't think we should turn it on for main tho cause then we'll get ugly "Github bot" commits on main.

Alternative is we turn that workflow off and use a githook like husky. But I find that people may get impatient and may force skip the pre commit hook if they need to commit quickly.

[Teddarific]

@alexdanilowicz

1. It should be straight forward changing the prettier - I think just deleting the file should do the trick. The big thing will just be a big commit reformatting all the files, but it shouldn't have to be done manually. If we don't already, we should have a command like yarn lint to lint the whole project and --fix any easy things

2. I mean looking at the use cases of any, there's only one instance where it's the boiler plate, which is inside of assets.d.ts right? That use case is legitimate, and we can just ts-ignore/ eslint-disable-next-line it. I don't feel strongly, but I don't see why not turn these on now if we're going to want to turn these on eventually (which IMO we should). We don't have to fix all the instances now - we can just ts-ignore the current instances if you don't want to fix them now which is totally fine, but it makes it so that if you REALLY need to use any, you have to be very conscious of doing it since you also have to add a ts-ignore

3. Yeah totally agreed - probably shouldn't turn it on for main. I've been pushing directly to main because I've been linting before I push and once we finish setting up monorepo, i'll start doing PRs but I'm too lazy to right now. This conversation deserves a separate thread, but tldr I changed the default merge to do a rebase, aka no squash. I guess we should do squash huh. I can change it back. But also, I think it would be nice to have a pre-git hook to run the linting locally before pushing, and not depend on a bot to commit for us. Not urgent, but a nice to have for the future so you don't have to pull the bot's changes down.

I'm going to look into hoisting prettier and eslint config for App right now, but I'll leave specific electron eslint configs as is and let you take care of 2)

[alexdanilowicz]

Ok, agreed on all your points. Thanks for typing that out.

re: bot vs git hook. Yeah con of bot is you have to pull down changes, but con of pre-commit hook is you have to wait for it to finish or (if it fails and you're impatient) you run --no-verify. I feel like it's an emacs vs vim thing. We can do whatever you want.

I'll handle # 2, sounds like you are doing # 1.

re: # 3 — squash or bust. (Why do you like rebase? I'm curious? I feel like it clutters commit history.)

[Teddarific]

I'm going to close this issue.