Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup whole codebase #542

Closed
wants to merge 0 commits into from
Closed

Conversation

Pringlers
Copy link

@Pringlers Pringlers commented Mar 2, 2023

Please squash merge it to remove inconsistent commit messages later

Goals

This PR is intended to clean up the entire codebase.

  • Create, modify, or remove the classes for better maintainability
  • Adopt more efficient logic
  • Remove duplicated code
  • Write idiomatic TypeScript

Non-Goals

  • Change website appearance
  • Create API v3
  • Change DB structure

utils/Query.ts Outdated Show resolved Hide resolved
utils/Query.ts Outdated Show resolved Hide resolved
utils/Query.ts Outdated Show resolved Hide resolved
utils/Query.ts Outdated Show resolved Hide resolved
@skinmaker1345 skinmaker1345 marked this pull request as ready for review March 3, 2023 16:44
@Pringlers
Copy link
Author

@skinmaker1345 There are more changes to be made in this PR, or do you want me to split this into multiple PRs?

@skinmaker1345 skinmaker1345 marked this pull request as draft March 3, 2023 16:55
@Pringlers
Copy link
Author

Type rework fixed every relevant type error.

image

Most of the changes in components are as simple as removing type assertions (e.g. (bot.users as User[])).
Components that only operate on partial data now require the type of XXX<true|false>

@Pringlers
Copy link
Author

XXXTable – 1:1 mapping of SQL query result. JSON fields are not parsed yet
BaseXXX – Same, but JSON fields are parsed
XXX<Nested> – Nested indicates whether it has fully resolved children or snowflakes.

@skinmaker1345
Copy link
Member

Looks great, except
@Pringlers your code seems to fetch nested objects, always, even for raw objects

@skinmaker1345
Copy link
Member

image

we are aware of that, what's the ts version you're running., by chance?

@Pringlers
Copy link
Author

Pringlers commented Mar 4, 2023

your code seems to fetch nested objects, always, even for raw objects

Can you point out where it does? I moved code to resolve children into getXXX, and _rawXXX DataLoaders now call getNestedXXX(id) directly if you mean that

we are aware of that, what's the ts version you're running., by chance?

4.9.5

@skinmaker1345
Copy link
Member

skinmaker1345 commented Mar 4, 2023

Can you point out where it does? I moved code to resolve children into getXXX, and _rawXXX DataLoaders now call getNestedXXX(id) directly if you mean that

think i confused each other, you did refactor as expected, apologies.

4.9.5

using 4.6.x resolves the issue(if you're having trouble with it), 4.6.4 is the version which we specify to use

@skinmaker1345
Copy link
Member

@Pringlers Do you have any works to be done?

@Pringlers
Copy link
Author

Yes I do, but it might take long to get them done. So feel free to merge it if you want to work on top of these changes

@skinmaker1345
Copy link
Member

skinmaker1345 commented Mar 7, 2023 via email

@skinmaker1345
Copy link
Member

Hi, I am afraid to tell that this pr should be closed as we are preparing to apply a formatter for entire codebase.

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