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

Implement auth and transfer commands #1298

Merged
merged 11 commits into from
Jan 17, 2025
Merged

Implement auth and transfer commands #1298

merged 11 commits into from
Jan 17, 2025

Conversation

f-f
Copy link
Member

@f-f f-f commented Nov 1, 2024

We fix #1123 by adding the auth and registry transfer commands, to allow for authenticated interactions with the Registry. The unpublish command is still unimplemented, but it should be easy now that the rest of the scaffolding is in place.

This PR includes a whole set of tests to verify that things do what they should, and docs to inform users how the authenticated flow looks like.

@fsoikin I intend to merge this before #1296 since I've been rebasing it for months already and I'd love to not have to do it once more 😄

(this fixes #1294 as well)

@f-f f-f requested a review from fsoikin November 1, 2024 22:51
@fsoikin
Copy link
Collaborator

fsoikin commented Nov 2, 2024

Absolutely no problem, I assumed it would be this way. I'm happy to keep rebasing until it's time.

@fsoikin
Copy link
Collaborator

fsoikin commented Nov 2, 2024

I'm afraid it'll take me a few days to review though. This is quite large.

@f-f
Copy link
Member Author

f-f commented Nov 3, 2024

@fsoikin no worries! That'll give me some time to have a good look at #1296 🙂

@f-f
Copy link
Member Author

f-f commented Nov 3, 2024

Of course happy to have @thomashoneyman's review as well here if it doesn't take away time from other things 🙂

Copy link
Member

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

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

This looks good to me! I don't know if I have enough fresh context on Spago to give an approval, but everything seems reasonable. Just a couple comments.

bin/src/Main.purs Outdated Show resolved Hide resolved
src/Spago/Command/Auth.purs Show resolved Hide resolved
src/Spago/Command/Fetch.purs Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@fsoikin fsoikin left a comment

Choose a reason for hiding this comment

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

Looks fluffy!

src/Spago/Command/Auth.purs Show resolved Hide resolved
src/Spago/Command/Auth.purs Outdated Show resolved Hide resolved
src/Spago/Command/Auth.purs Outdated Show resolved Hide resolved
src/Spago/Command/Publish.purs Outdated Show resolved Hide resolved
src/Spago/Command/Registry.js Outdated Show resolved Hide resolved
test-fixtures/1146-cli-help/build.txt Show resolved Hide resolved
@f-f f-f mentioned this pull request Jan 17, 2025
f-f and others added 8 commits January 17, 2025 13:58
Co-authored-by: Thomas Honeyman <admin@thomashoneyman.com>
Co-authored-by: Fyodor Soikin <name.fa@gmail.com>
Co-authored-by: Fyodor Soikin <name.fa@gmail.com>
Co-authored-by: Fyodor Soikin <name.fa@gmail.com>
Co-authored-by: Fyodor Soikin <name.fa@gmail.com>
@f-f f-f merged commit d9ef296 into master Jan 17, 2025
5 checks passed
@f-f f-f deleted the transfer-operation branch January 17, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants