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

Rename Buildpackage to PackageDescriptor #656

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Conversation

Malax
Copy link
Member

@Malax Malax commented Aug 24, 2023

While https://buildpacks.io refers to package.toml as a "buildpackage" once, it is usually referred to as package.toml which is what we also use in our day-to-day conversations. I think Buildpackage is a poor name to use within libcnb for the following reasons:

  • It starts with "buildpack", so it turns up in searches for "buildpack".
  • We also currently have BuildpackPackage, it's very easy to confuse the two.
  • The name in general is quite similar to other names we have since it starts with "build".

I wanted to avoid renaming it to PackageToml. Even though it might be the first thing that comes to mind as better name, I don't think its a good one. The Toml suffix would imply it is TOML data which is not the case for the parsed representation. It can be created via code and never be (de)serialized from or to TOML and the serialised representation is irrelevant when we work with the data in code. We have a similar situation with buildpack.toml which we named BuildpackDescriptor for similar reasons.

Thus, I went with PackageDescriptor to align with BuildpackDescriptor. I think readability of the code already improved after this rename.

I have other changes in mind with regard to PackageDescriptor that should simplify the code a bunch, but these are out of scope for this PR. This is strictly just a rename.

Ref: GUS-W-14007685

CHANGELOG.md Show resolved Hide resolved
@Malax Malax marked this pull request as ready for review August 24, 2023 10:28
@Malax Malax requested a review from a team as a code owner August 24, 2023 10:28
@Malax Malax enabled auto-merge (squash) August 24, 2023 10:57
@Malax Malax merged commit ea5103e into main Aug 24, 2023
8 checks passed
@Malax Malax deleted the malax/rename-buildpackage branch August 24, 2023 11:17
@Malax Malax mentioned this pull request Aug 29, 2023
edmorley added a commit that referenced this pull request Sep 21, 2023
Now that we're treating `libcnb-package` APIs as internal, we're not
mentioning `libcnb-package` API changes in the changelog.

As such, the changes within #656 can now be described purely in
terms of the `libcnb-data` changes, making the changelog entry
consistent with other libcnb-data changelog entries in the upcoming
release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants