Skip to content

Conversation

@ichbinder
Copy link
Contributor

@ichbinder ichbinder commented Feb 15, 2025

I have added a yes option (-y) because otherwise I will be asked in automatic scripts whether I want to create a cert, for example.
I haven't tested it completely yet, I'll do that tomorrow.

@Erreur32
Copy link
Owner

Erreur32 commented Feb 16, 2025

Thank you for your contribution!

While most of your changes are good and helpful (especially the -y option for automation), but i need to reject one part of the PR:

The line 1360:
HTTP_RESPONSE=$(curl -s -w "HTTPSTATUS:%{http_code}" -X POST "$BASE_URL/nginx/certificates" \

This is incorrect because:
It uses a different variable format than the rest of the script.
I don't know you have changed this ;)

Should be:
HTTP_RESPONSE=$(curl -s -w "HTTPSTATUS:%{http_code}" -X POST "$BASE_URL/users" \

Would you mind submitting an updated PR without the HTTP_RESPONSE changes? The rest of your improvements would be very welcome additions to the project.

( Also, I initially planned to allow auto-accept for scripts but also to clean up the output so that only the minimum necessary information is displayed. )

@Erreur32 Erreur32 self-assigned this Feb 16, 2025
…er-Bash-API into addYesOption # Conflicts: #	nginx_proxy_manager_cli.sh
@ichbinder
Copy link
Contributor Author

Hello,

Thank you for your constructive feedback. I have updated the PR accordingly and reverted the changes to the HTTP_RESPONSE line as requested. I’m glad you found the other improvements, especially the -y option for automation, useful.

Please let me know if you have any further suggestions or improvements.

Copy link
Owner

@Erreur32 Erreur32 left a comment

Choose a reason for hiding this comment

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

I will clean some German text after validiting ;)
And i will update README as well .

@Erreur32 Erreur32 merged commit dd390ff into Erreur32:main Feb 16, 2025
@ichbinder
Copy link
Contributor Author

Great thanks, I'm already using it and it seems to work.

@Erreur32
Copy link
Owner

I will check also when i time, but code should be good as i sow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants