Skip to content

Conversation

pawelangelow
Copy link
Collaborator

@pawelangelow pawelangelow commented Jan 11, 2025

Description

Well, I noticed few files with no formatting, random code style issues, so here's a solution to the problems.

In this PR you're going to see prettier + configuration for it.

⚠️ Nothing is enforced (there's no pre-commit hook, no CI pipeline, etc...). The idea is when you work on a file and want to format it - to use some defined rules and not spend time on manual edits. Current configuration is enhanced from the repo code style - the idea is to change as less as possible.

In addition I'm adding a VSCode configuration to trigger the prettier rules when formatting files. (if you haven't changed your binding it should be cmd + shift + f.

How to test?

yarn lint

and

yarn prettier or yarn prettier:update

@seth-riedel
Copy link
Contributor

I'm all for this change, but it's going to cause a lot of problems if we try to do this while maintaining two separate long-lived branches that we constantly merge into one another

pd-redis
pd-redis previously approved these changes Jan 14, 2025
parser: '@typescript-eslint/parser',
rules: {
semi: ['error', 'never'],
semi: ['error', 'always'],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this? It will probably cause a lot of changes, that could be avoided

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because I needed to override it. Currently we don't put ; on .ts files, which is OK. However, it's not the best practice for .js files as there are cases where problems may occur:

function test() {
  return
  {
    message: "Hello, world!"
  }
}

console.log(test())

So, below in this file are the override settings.

@pawelangelow
Copy link
Collaborator Author

I'm all for this change, but it's going to cause a lot of problems if we try to do this while maintaining two separate long-lived branches that we constantly merge into one another

I agree that this change could introduce some challenges, but once we merge and rebase the branches, I believe things will stabilize moving forward. Could you share more about your specific concerns or scenarios where you foresee issues? That way, we can address them proactively.

@pawelangelow pawelangelow merged commit 86956ef into main Mar 4, 2025
4 checks passed
@pawelangelow pawelangelow deleted the linting branch March 4, 2025 14:11
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.

3 participants