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

fix(oci): create blobs dir and define blobs dir name constant #520

Merged
merged 1 commit into from
Jun 25, 2023

Conversation

sparr
Copy link
Contributor

@sparr sparr commented May 31, 2023

https://github.com/opencontainers/image-spec/blob/v1.0/image-layout.md#content specifies that the blobs dir must exist, so the changed ensureDir in this PR is a bugfix.

The addition and use of the ociBlobsDir constant is more of a cleanup suggestion, and I can remove it if necessary. I've also asked for it to be added upstream at opencontainers/image-spec#1069 but that will probably take significantly longer.

Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM, but I am not a maintainer

Copy link
Member

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

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

/lgtm

@sparr
Copy link
Contributor Author

sparr commented Jun 2, 2023

Thank you both.

@TerryHowe, can you approve running the check workflows?

@sparr
Copy link
Contributor Author

sparr commented Jun 2, 2023

Created opencontainers/image-spec#1071 which would make the new constant here unnecessary.

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2023

Codecov Report

Merging #520 (ac4d2e2) into main (b4400e1) will not change coverage.
The diff coverage is 50.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main     #520   +/-   ##
=======================================
  Coverage   73.22%   73.22%           
=======================================
  Files          49       49           
  Lines        4602     4602           
=======================================
  Hits         3370     3370           
  Misses        923      923           
  Partials      309      309           
Impacted Files Coverage Δ
content/oci/oci.go 75.28% <0.00%> (ø)
content/oci/readonlystorage.go 87.50% <100.00%> (ø)

@sparr
Copy link
Contributor Author

sparr commented Jun 23, 2023

opencontainers/image-spec#1071 is merged. If they have a release before this PR is merged, I'll switch over to using the blobs constant from there, ditto for ociImageIndexFile

@shizhMSFT
Copy link
Contributor

opencontainers/image-spec#1071 is merged. If they have a release before this PR is merged, I'll switch over to using the blobs constant from there, ditto for ociImageIndexFile

Cool. Let's wait for image-spec v1.1.0-rc.4 with your commits in there.

@shizhMSFT shizhMSFT added this to the v2.3.0 milestone Jun 25, 2023
@shizhMSFT shizhMSFT changed the title Create blobs dir. Define blobs dir name constant. fix: create blobs dir and define blobs dir name constant Jun 25, 2023
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM. We may merge this PR if the next release of image-spec takes too long.

@shizhMSFT shizhMSFT changed the title fix: create blobs dir and define blobs dir name constant fix(oci): create blobs dir and define blobs dir name constant Jun 25, 2023
@sparr
Copy link
Contributor Author

sparr commented Jun 25, 2023

Sadly I missed the cutoff for rc4 by just a few hours. For unrelated reasons I've been talking to the maintainers over there about their releases, and it's unclear whether rc5 is days or months away, but weeks seems most likely.

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
@TerryHowe TerryHowe merged commit 2e7b65f into oras-project:main Jun 25, 2023
7 checks passed
@sparr sparr deleted the blobs_dir branch June 27, 2023 17:51
Wwwsylvia pushed a commit to Wwwsylvia/oras-go that referenced this pull request Jul 4, 2023
…roject#520)

https://github.com/opencontainers/image-spec/blob/v1.0/image-layout.md#content
specifies that the blobs dir must exist, so the changed ensureDir in
this PR is a bugfix.

The addition and use of the ociBlobsDir constant is more of a cleanup
suggestion, and I can remove it if necessary. I've also asked for it to
be added upstream at
opencontainers/image-spec#1069 but that will
probably take significantly longer.

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
shizhMSFT pushed a commit that referenced this pull request Jul 4, 2023
https://github.com/opencontainers/image-spec/blob/v1.0/image-layout.md#content
specifies that the blobs dir must exist, so the changed ensureDir in
this PR is a bugfix.

The addition and use of the ociBlobsDir constant is more of a cleanup
suggestion, and I can remove it if necessary. I've also asked for it to
be added upstream at
opencontainers/image-spec#1069 but that will
probably take significantly longer.

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
@Wwwsylvia Wwwsylvia mentioned this pull request Jul 4, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants