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

feat: fix quality criteria issues gf-557 #561

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

Vitaliistf
Copy link
Collaborator

Fixed:

  • Inconsistent naming. Folder and file name did not correspond to hook name.
  • Spelling mistakes.
  • Inconsistent orderBy columns names case. Changed them to camelCase.
  • Inconsistent file structure (helper has its own folder).
  • Inconsistent icon filling.

Besides fixes that were described in ticket, I've changed case of all table and column names in objection queries (except for raw parts of queries of course).

@@ -1,3 +1,3 @@
<svg width="18" height="18" viewBox="0 0 18 18" fill="none" xmlns="http://www.w3.org/2000/svg">
<svg width="18" height="18" viewBox="0 0 18 18" fill="currentColor" xmlns="http://www.w3.org/2000/svg">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have only one question. Why do we need to replace fill="none"?

Copy link
Collaborator Author

@Vitaliistf Vitaliistf Sep 27, 2024

Choose a reason for hiding this comment

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

It's just for consistency. Either "currentColor" or "none". I chose "currentColor" because some icons don't have fill in path tag. Maybe it will be not so confusing as it would be if we had "none" (and no path).

@liza-veis
Copy link
Collaborator

Could you also please fix OrderKey from @GvoFor pull request?

@Vitaliistf
Copy link
Collaborator Author

Could you also please fix OrderKey from @GvoFor pull request?

Oh, yeah, sorry, I've changed it but forgot to add to commit😅

@liza-veis liza-veis merged commit 6b3c9ed into main Sep 27, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

4 participants