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

Make where we work with BOSH Release tarballs explicit #405

Merged

Conversation

crhntr
Copy link
Member

@crhntr crhntr commented Jun 28, 2023

TL;DR

  • component.Spec and cargo.ComponentSpec -> cargo.BOSHReleaseTarballSpecification
  • component.Lock and cargo.ComponentLock -> cargo.BOSHReleaseTarballLock
  • cargo.ReleaseSourceTypeBOSHIO -> cargo.BOSHReleaseTarballSourceTypeBOSHIO
  • cargo.ReleaseSourceTypeS3 -> cargo.BOSHReleaseTarballSourceTypeS3
  • cargo.ReleaseSourceTypeGithub -> cargo.BOSHReleaseTarballSourceTypeGithub
  • cargo.ReleaseSourceTypeArtifactory -> cargo.BOSHReleaseTarballSourceTypeArtifactory
  • a few other function and method names were changed in a similar way to align with the above rename operations.

Discussion

This refactor is intended to make it easier to think about and code around the BOSH releases tarballs we package.

  • In 96032c8, I merge component.Lock and cargo.ComponentLock. They have literally been the same thing for a while; this commit removes the indirection.
    • component.Local and component.Exported no longer embed the cargo.ComponentLock but have it as a non-embedded field. This makes refactors easier and makes it clear that those types should not inherit methods from the Lock. For the most part, this change is reversible in a non-breaking way.
  • In a67fdfd, I rename "Component{Lock,Spec} to BOSHRelease{Lock,Spec}. We may still want to refer to BOSH Releases as tile components in speech and docs but the code reads better when it is less ambiguous about what is is operating on.
  • In f0f1656, I add the Tarball part to the BOSHRelease names.
  • In 7977710, I renamed other public identifiers and methods to stay consistent with the Tarball Lock and Spec types.

they were already an alias, this removes that indirection
this makes it clearer what is being worked with
add Tarball and make it clear we are working with BOSH releases
@cf-gitbot
Copy link
Member

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

@crhntr crhntr changed the title Make clear places where we are working with BOSHReleaseTarballs Make it clear where we are working with BOSH Release tarballs Jun 28, 2023
@crhntr
Copy link
Member Author

crhntr commented Jun 28, 2023

Here is some "prior art". PR #405 is bigger because it merges and renames; the prior art commit does not merge the types it just renames cargo type.

@crhntr crhntr added go Pull requests that update Go code ready-for-review labels Jun 28, 2023
@crhntr crhntr changed the title Make it clear where we are working with BOSH Release tarballs Make clear where we work with BOSH Release tarballs Jun 28, 2023
@crhntr crhntr changed the title Make clear where we work with BOSH Release tarballs Make where we work with BOSH Release tarballs explicit Jun 28, 2023
@crhntr crhntr requested a review from staylor14 July 6, 2023 19:00
Copy link
Contributor

@dtimm dtimm left a comment

Choose a reason for hiding this comment

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

Excellent refactor. LGTM.

@crhntr crhntr merged commit f838491 into main Jul 7, 2023
3 checks passed
@crhntr crhntr deleted the feature/improve-public-api-identifiers-for-bosh-release-tarballs branch July 7, 2023 17:06
@crhntr crhntr removed the request for review from staylor14 July 7, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code ready-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants