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

fix: remove attestation-name input and output #3313

Merged
merged 6 commits into from
Mar 13, 2024
Merged

fix: remove attestation-name input and output #3313

merged 6 commits into from
Mar 13, 2024

Conversation

ramonpetgrave64
Copy link
Collaborator

@ramonpetgrave64 ramonpetgrave64 commented Mar 8, 2024 •
edited
Loading

Summary

Testing Process

We cannot properly test workflow changes without first merging to main, and revert if tests fail. After merging we will

  • trigger a manual run of pre-submit e2e generic default
  • wait for nightly e2e tests to report success

Checklist

  • Review the contributing guidelines
  • Add a reference to related issues in the PR description.
  • Update documentation if applicable.
  • Add unit tests if applicable.
  • Add changes to the CHANGELOG if applicable.

@ramonpetgrave64
remove attestation-name input
5ee0f0f 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
rename intenral variables, to be safe
66f0fe3 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
makrdownlint
9b15ff5 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64 ramonpetgrave64 marked this pull request as ready for review March 8, 2024 19:22
@ramonpetgrave64
remove uneeded changes
3350404 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
clarify changes
984817d 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
laurentsimon
laurentsimon reviewed Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

nit: can you edit the CHANGELOG

@ramonpetgrave64
Copy link
Collaborator Author

@laurentsimon Yeah I did edit the changelog, but not as a breaking change since we're removing an already deprecated thing.

@laurentsimon
Copy link
Collaborator

laurentsimon commented Mar 13, 2024 •
edited
Loading

@laurentsimon Yeah I did edit the changelog, but not as a breaking change since we're removing an already deprecated thing.

Let's make it breaking, because some folks are still using it. Wdut?

@ramonpetgrave64
make it a breaking change
c95cd16 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
Copy link
Collaborator Author

Let's make it breaking, because some folks are still using it. Wdut?

Alright

@laurentsimon laurentsimon merged commit e64a212 into slsa-framework:main Mar 13, 2024
74 checks passed
@ianlewis
Copy link
Member

@laurentsimon Yeah I did edit the changelog, but not as a breaking change since we're removing an already deprecated thing.

I think making it breaking is a good call because removing something that's deprecated is a breaking change regardless of whether it was deprecated or not. Deprecated just means that it will be removed (in a major release) in the future.

This was referenced Mar 15, 2024
Closed
Merged
ramonpetgrave64 added a commit that referenced this pull request Mar 20, 2024
@ramonpetgrave64
Revert "fix: remove attestation-name input and output (#3313)"
d5ae2e5 
This reverts commit e64a212.
@laurentsimon laurentsimon mentioned this pull request Mar 20, 2024
Closed
laurentsimon added a commit that referenced this pull request Mar 20, 2024
@laurentsimon
Revert "fix: remove attestation-name input and output (#3313)"
1794ca5 
This reverts commit e64a212.
@laurentsimon laurentsimon mentioned this pull request Mar 20, 2024
Merged
ramonpetgrave64 added a commit that referenced this pull request Mar 20, 2024
@ramonpetgrave64
Revert "fix: remove attestation-name input and output (#3313)"
21b5190 
This reverts commit e64a212.
ramonpetgrave64 added a commit that referenced this pull request Mar 20, 2024
@ramonpetgrave64
Revert "fix: remove attestation-name input and output (#3313)"
82a0bd0 
This reverts commit e64a212.

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
laurentsimon added a commit that referenced this pull request Mar 21, 2024
@laurentsimon
chore: Revert "fix: remove attestation-name input and output" (#3399)
2cf77fa 
Reverts #3313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentsimon laurentsimon laurentsimon approved these changes

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[feature] attestation-name deprecation warning [bug] Opaque failure in final step
3 participants
@ramonpetgrave64 @laurentsimon @ianlewis