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.

Sign up for GitHub

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

Jump to bottom

update: removed Sign with OCI artifact manifest #308

Merged
merged 3 commits into from
May 11, 2023
Merged

update: removed Sign with OCI artifact manifest #308

merged 3 commits into from
May 11, 2023

Conversation

Two-Hearts
Copy link
Contributor

This PR removes notation-go Sign's support of OCI artifact manifest as it is removed from image-spec v1.0.0-rc.3. In other words, Notation will only generate signatures stored as OCI image manifest in the future.
(Note: Notation can still consume signatures already stored as OCI artifact manifest for backwards compatibility.)

Related issue: notaryproject/notation#660

shizhMSFT
shizhMSFT previously approved these changes May 10, 2023
View reviewed changes
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

@shizhMSFT shizhMSFT mentioned this pull request May 10, 2023
Merged
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2023 •
edited
Loading

Codecov Report

Merging #308 (80ec48f) into main (06d04cc) will not change coverage.
The diff coverage is 16.66%.

❗ 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     #308   +/-   ##
=======================================
  Coverage   74.39%   74.39%           
=======================================
  Files          23       23           
  Lines        2175     2175           
=======================================
  Hits         1618     1618           
  Misses        439      439           
  Partials      118      118
Impacted Files Coverage Δ
registry/repository.go 49.35% <16.66%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

JeyJeyGao
JeyJeyGao previously approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

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

LGTM

priteshbandi
priteshbandi reviewed May 10, 2023
View reviewed changes
Copy link
Contributor

@priteshbandi priteshbandi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you please update below references of artifact manifest?

notation-go/registry/interface.go

Line 17 in f9c78f5

// artifact manifest descriptor

notation-go/internal/mock/mocks.go

Line 76 in f9c78f5

MediaType: "application/vnd.cncf.oras.artifact.manifest.v1+json",

@Two-Hearts Two-Hearts dismissed stale reviews from JeyJeyGao and shizhMSFT via 80ec48f May 11, 2023 02:56
JeyJeyGao
JeyJeyGao approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

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

LGTM

priteshbandi
priteshbandi approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

shizhMSFT
shizhMSFT approved these changes May 11, 2023
View reviewed changes
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

shizhMSFT

This comment was marked as duplicate.

Sign in to view

@shizhMSFT shizhMSFT merged commit 4e790ce into notaryproject:main May 11, 2023
@Two-Hearts Two-Hearts deleted the rc5 branch May 11, 2023 07:17
@yizha1 yizha1 added this to the v1.0.0-rc.5 milestone May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@priteshbandi priteshbandi priteshbandi approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@JeyJeyGao JeyJeyGao JeyJeyGao approved these changes

@gokarnm gokarnm Awaiting requested review from gokarnm gokarnm is a code owner

@justincormack justincormack Awaiting requested review from justincormack

@NiazFK NiazFK Awaiting requested review from NiazFK NiazFK is a code owner

@patrickzheng200 patrickzheng200 Awaiting requested review from patrickzheng200

@rgnote rgnote Awaiting requested review from rgnote rgnote is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

Assignees
No one assigned
Labels
None yet
Projects
Notary Project Planning Board
Status: No status
Milestone
v1.0.0-rc.5
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Two-Hearts @codecov-commenter @priteshbandi @shizhMSFT @JeyJeyGao @yizha1