Skip to content

Conversation

buccarel
Copy link
Contributor

@buccarel buccarel commented Dec 6, 2023

This is a proposal to fix the following issue:
#456

The rationale behind this is based on what @migmartri said in the issue, i.e.:

it's likely that we'll move away from setting current in the backend and instead be the client who sets it

I think this could be a step in the right direction, because the limitation is happening in the CLI command only.

I also noticed that there are no tests for these implementations, and I think I understand why that is. I think for the moment we could rely on end-to-end, before merging this. Test:
https://pastebin.com/HeGXrewd

Signed-off-by: Mattia Buccarella <m.buccarella@gmail.com>
@buccarel buccarel requested a review from migmartri December 6, 2023 13:22
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

LGTM, ptal at my comment about the error message to be returned.

Signed-off-by: Mattia Buccarella <m.buccarella@gmail.com>
@buccarel buccarel merged commit 8ae1211 into chainloop-dev:main Dec 6, 2023
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.

2 participants