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

Remove the api package #1055

Merged
merged 40 commits into from
Oct 6, 2023
Merged

Remove the api package #1055

merged 40 commits into from
Oct 6, 2023

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Sep 29, 2023

What?

  • Suffixes the api pkg types with API and fixes the linter errors (commits that start with "Rename".)
  • Moves the API types into the common pkg (commits that start with "Move" and "Use".)
  • Removes the api pkg for good (commits that start with "Remove".)

Why?

  • We postfix the api types with API to distinguish them from the common types. So, we can move them into the common pkg.
  • Moving them into the common pkg is necessary because there are many dependencies to these types throughout the module, and we can only sanely refactor these into concrete types within the common pkg.
  • In another PR, we can turn these interfaces into concrete types now that we don't have an api pkg dependency.

See the other reasonings in #898.

Checklist

  • I have performed a self-review of my code
  • I have added tests for my changes
  • I have commented on my code, particularly in hard-to-understand areas

Related PR(s)/Issue(s)

Updates: #898 (Actually, it finishes the work, but it's better to move the interfaces to concrete types to realize the issue's goal fully).

@inancgumus inancgumus self-assigned this Sep 29, 2023
@inancgumus inancgumus marked this pull request as ready for review September 29, 2023 11:16
@inancgumus inancgumus requested a review from ka3de September 29, 2023 11:16
@inancgumus inancgumus removed the request for review from ka3de September 29, 2023 12:52
@inancgumus inancgumus requested review from ka3de and ankur22 September 29, 2023 12:59
@inancgumus inancgumus changed the title Rename api pkg types by suffixing with API Remove the api package Oct 2, 2023
@inancgumus inancgumus force-pushed the refactor/api-package branch 2 times, most recently from 72b7463 to f80a2a6 Compare October 2, 2023 12:23
@inancgumus inancgumus force-pushed the refactor/api-package branch from f80a2a6 to 9634828 Compare October 2, 2023 12:24
Copy link
Collaborator

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for starting on this. I think in the long run this will make things more maintainable and easier to navigate around the project.

@inancgumus inancgumus merged commit 733e86a into main Oct 6, 2023
16 checks passed
@inancgumus inancgumus deleted the refactor/api-package branch October 6, 2023 07:17
@inancgumus inancgumus added the productivity Issues and PRs that improve our productivity label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
productivity Issues and PRs that improve our productivity refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants