Open Splamy opened 3 months ago
First thank you for the PR! It will be a valuable addition to the project.
I haven't run or checked the PR yet, but what caught my eye is the documentation.
First I have to say good job. Good use of the Tabs feature 😀
I think this addition justifies a separate page under the Configuration
category to prevent cluttering the page.
So my suggestion is the following structure:
The new page should contain the existing documentation for this feature and the new additional configuration.
To answer one of your questions: I personally prefer one class/enum/struct per file.
This is awesome, excellent work!
I would consider allowing Windows user to opt-in to encrypting their secret using DPAPI, but definitely not a blocker to landing this.
Thanks for the feedback, will try to address as soon as I can.
This is awesome, excellent work!
I would consider allowing Windows user to opt-in to encrypting their secret using DPAPI, but definitely not a blocker to landing this.
Interesting idea. Adding this will also need to have a proper ux for that. Either a cli command creating the hashed pw, or creating and updating the field in the appsettings (though appsettings aren't supposed to be automatically written since you will lose formatting and comments).
Maybe ./BaGetter.exe encrypt-password pass123
to stdout like caddy does
I think I've adressed everything including the feedback from @FroggieFrog.
Your original issue was the authentication against a GitHub feed if I remember correctly. So we could test it by releasing BaGetter packages on a GitHub feed 😬
For using as a test feed yeah, for actual distribution more only as a gimmick :P
I think there is nothing wrong when we publish BaGetter on NuGet and here on GitHub. At least from my point of view, there is nothing to be said against it.
Sorry for the late update. Somehow oversaw the review response. Updated the check and rebased to latest main.
Addresses https://github.com/orgs/bagetter/discussions/88
Open questions: