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

prefer AirbyteVersion over String #7310

Merged
merged 3 commits into from
Oct 26, 2021
Merged

Conversation

cgardens
Copy link
Contributor

What

  • We have an abstraction called AirbyteVersion to make it easy to interact with versions and perform various operations on them. Throughout the codebase though, we tend pass the version around as a String. We need to lean into this abstraction. It gives us type safety guarantees. It also allows us to push noisy serialization issues out to the edge of our system. With the string, there were a lot of odd checks to make sure it was not empty and such deep in our internals.
  • Going forward we should be pretty vigilant about passing AirbyteVersion instead of String.

How

  • Search and destroy. Find String usages, replace with AirbyteVersion
  • I skipped the airbyte-migrations submodule as that was going to be a real pain, and we plan to remove it soon anyway.

@cgardens cgardens force-pushed the cgardens/leverage-airbyte-version branch from 90dccb2 to 72cf4d7 Compare October 26, 2021 00:21
@cgardens cgardens temporarily deployed to more-secrets October 26, 2021 00:23 Inactive
@cgardens cgardens merged commit 35e9050 into master Oct 26, 2021
@cgardens cgardens deleted the cgardens/leverage-airbyte-version branch October 26, 2021 01:06
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants