Unify pre-commit config across repos #1006

dhruvkb · 2022-11-11T06:21:14Z

Fixes

Addresses WordPress/openverse#316 by @dhruvkb

Description

This PR uses the new pre-commit config introduced in WordPress/openverse#330 and fixes the code so that the lint checks pass.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the default branch of the repository (main) or a parent feature branch.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

github-actions · 2022-11-11T06:22:39Z

API Developer Docs Preview: Ready

https://wordpress.github.io/openverse-api/_preview/1006

Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again.

You can check the GitHub pages deployment action list to see the current status of the deployments.

obulat

Cleaner look is always welcome :)

obulat · 2022-11-11T06:23:22Z

api/catalog/api/controllers/search_controller.py

@@ -52,7 +52,7 @@ def _unmasked_query_end(page_size, page):

 def _paginate_with_dead_link_mask(
    s: Search, page_size: int, page: int
-) -> Tuple[int, int]:
+) -> tuple[int, int]:


Interesting that it replaces the types this way!

I like that it saves imports 😄

obulat · 2022-11-11T06:33:18Z

.pre-commit-config.yaml

@@ -19,6 +21,7 @@ repos:
        args:
          - --remove
      - id: pretty-format-json
+        exclude: package(-lock)?\.json


Would this affect tsconfig.json in any way?

This isn't being synced with the frontend yet. There are some repo-specific hooks in the frontend so syncing them centrally is not feasible (yet).

krysal

Love the syntax standardization across Repos ✨

krysal · 2022-11-11T16:04:19Z

.git-blame-ignore-revs

@@ -0,0 +1,2 @@
+# Add pyupgrade
+5158c579d0c8eeb6e9372b30a03ce48f53459958


I wonder if this does something after squashing 💭

You're completely right, that'll not work!

krysal · 2022-11-11T16:05:51Z

api/catalog/api/controllers/search_controller.py

@@ -52,7 +52,7 @@ def _unmasked_query_end(page_size, page):

 def _paginate_with_dead_link_mask(
    s: Search, page_size: int, page: int
-) -> Tuple[int, int]:
+) -> tuple[int, int]:


I like that it saves imports 😄

krysal · 2022-11-11T16:07:32Z

api/catalog/api/utils/help_text.py

-from typing import Iterable
+from collections.abc import Iterable


Interesting change 😮

krysal · 2022-11-11T16:11:04Z

api/test/dead_link_filter_test.py

-    res_2_ids = set(result["id"] for result in data_without_dead_links["results"])
+    res_1_ids = {result["id"] for result in data_with_dead_links["results"]}


Here I kind of like that the set() is more explicit as the {} could be confused by a dictionary if one doesn't pay attention, but I guess it is acceptable.

dhruvkb added 🟩 priority: low Low priority and doesn't need to be rushed ✨ goal: improvement Improvement to an existing user-facing feature 🤖 aspect: dx Concerns developers' experience with the codebase labels Nov 11, 2022

dhruvkb requested a review from a team as a code owner November 11, 2022 06:21

dhruvkb requested review from AetherUnbound and obulat November 11, 2022 06:21

dhruvkb mentioned this pull request Nov 11, 2022

Unify pre-commit config across repos WordPress/openverse#330

Merged

7 tasks

obulat approved these changes Nov 11, 2022

View reviewed changes

dhruvkb force-pushed the one_true_lint branch from f1d90e9 to ca2f455 Compare November 11, 2022 06:50

dhruvkb added 2 commits November 11, 2022 11:31

Improve pre-commit config and align with WordPress/openverse

5158c57

Exclude lint fixes from Git blame

c1c9e89

dhruvkb force-pushed the one_true_lint branch from ca2f455 to c1c9e89 Compare November 11, 2022 07:31

krysal approved these changes Nov 11, 2022

View reviewed changes

Remove hash to add later

fd3b8a7

dhruvkb merged commit ea2162f into main Nov 11, 2022

dhruvkb deleted the one_true_lint branch November 11, 2022 21:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify pre-commit config across repos #1006

Unify pre-commit config across repos #1006

dhruvkb commented Nov 11, 2022

github-actions bot commented Nov 11, 2022 •

edited

Loading

obulat left a comment

obulat Nov 11, 2022

krysal Nov 11, 2022

obulat Nov 11, 2022

dhruvkb Nov 11, 2022

krysal left a comment

krysal Nov 11, 2022

dhruvkb Nov 11, 2022

krysal Nov 11, 2022

krysal Nov 11, 2022

krysal Nov 11, 2022

		@@ -0,0 +1,2 @@
		# Add pyupgrade
		5158c579d0c8eeb6e9372b30a03ce48f53459958

		from typing import Iterable
		from collections.abc import Iterable

		res_2_ids = set(result["id"] for result in data_without_dead_links["results"])
		res_1_ids = {result["id"] for result in data_with_dead_links["results"]}

Unify pre-commit config across repos #1006

Unify pre-commit config across repos #1006

Conversation

dhruvkb commented Nov 11, 2022

Fixes

Description

Checklist

Developer Certificate of Origin

github-actions bot commented Nov 11, 2022 • edited Loading

obulat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krysal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 11, 2022 •

edited

Loading