Related to T4261
Details
Details
- Reviewers
anlambert - Group Reviewers
Reviewers - Maniphest Tasks
- T4261: Swh-graphql Code cleanup
- Commits
- rDGQLf5ef0a1f8b4c: Add type hints to resolver modules
Diff Detail
Diff Detail
- Repository
- rDGQL GraphQL API
- Branch
- types
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 31497 Build 49272: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 49271: arc lint + arc unit
Event Timeline
Comment Actions
Build is green
Patch application report for D8462 (id=30490)
Rebasing onto 7e4eacb71f...
Current branch diff-target is up to date.
Changes applied before test
commit 813e9243b611052f4f5a9d774248b1a08e9fa750
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 13 12:03:14 2022 +0200
Add type hints to resolver modules
Related to T4261See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/173/ for more details.
Comment Actions
Looks good to me, just one missing return type to add.
Reading you code more deeply, I think it would be better to get rid of the resolvers_factory module
as it adds a not really needed indirection and makes code hard to follow.
Returning the proper node object directly in the *_resolver functions seems simpler to me and
you can set a local mapping based on object type in them when returning an Union.
| swh/graphql/resolvers/resolvers.py | ||
|---|---|---|
| 288 | -> rs.revision.LogRevisionConnection | |
Comment Actions
Build is green
Patch application report for D8462 (id=30499)
Rebasing onto f2f222267f...
First, rewinding head to replay your work on top of it... Applying: Add type hints to resolver modules
Changes applied before test
commit 4011c0393be9cdbf3b29fb7d3a2f102382830089
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 13 12:03:14 2022 +0200
Add type hints to resolver modules
Related to T4261See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/175/ for more details.
Comment Actions
Thanks. I have a diff planned for resolver factory. I will replace that with a proper factory method.
The idea with the factory was to help a user get the resolver class without reading any other code.
But it got a bit complex later, I will simplify that.
Comment Actions
Build is green
Patch application report for D8462 (id=30503)
Rebasing onto f2f222267f...
Current branch diff-target is up to date.
Changes applied before test
commit f5ef0a1f8b4ca0335a0456f043c534cf5ffeac90
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 13 12:03:14 2022 +0200
Add type hints to resolver modules
Related to T4261See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/176/ for more details.