Skip to content

Conversation

@MalTeeez
Copy link

@MalTeeez MalTeeez commented May 9, 2025

This PR adds generalized style rules for all modules, to be adjusted as seems reasonable.

@vercel
Copy link

vercel bot commented May 9, 2025

@MalTeeez is attempting to deploy a commit to the freemanjiang's projects Team on Vercel.

A member of the Team first needs to authorize it.

@MalTeeez
Copy link
Author

MalTeeez commented May 9, 2025

No code actual modified yet, should probably be done once we are done with the rules.

@rubb3rDucc: I'd say the rules for server/ and shared/ are done, client/ is still in progress as we deem how much is reasonable.

Currently at 797 problems (766 errors, 31 warnings)

@MalTeeez
Copy link
Author

MalTeeez commented May 9, 2025

After correctly resolving local package imports, we are now at 258 problems (225 errors, 33 warnings), most of which happen in the client module because of missing semicolons.

@rubb3rDucc
Copy link

rubb3rDucc commented May 9, 2025

After correctly resolving local package imports, we are not at 258 problems (225 errors, 33 warnings), most of which happen in the client module because of missing semicolons.

Cool, in that case, I think it's fine because TS doesn't mind the semicolon being there or not. Unless @freeman-jiang does not want to keep the code without semicolons, you can put a flag to not be as strict for checking for semicolons.

I also looked at the committed files last night, I think this looks good so far. I'll pull it down later on.

@MalTeeez
Copy link
Author

Yeah, not sure if we want semicolons optional, but i wouldn't mind. I just inferred from the heavy usage in the server module

@MalTeeez
Copy link
Author

Yeah, not sure if we want semicolons optional, but i wouldn't mind. I just inferred from the heavy usage in the server module

Problem is, we need to decide on one variant (require everywhere, or forbid everywhere and rely on ASI), which, going by https://eslint.style/rules/js/semi , id say require everywhere seems more reasonable, and if possible lower the action to a warning instead of an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants