search

basecamp / thruster

MIT License
912 stars 31 forks source link

Deprecate `SSL_DOMAIN` in favor of `TLS_DOMAIN` #13

Closed 3v0k4 closed 7 months ago

3v0k4 commented 8 months ago

Fixes #7 (notice that also https://letsencrypt.org/ mentions TLS)

Thanks for creating Thruster!

It's fun and I'm learning a ton seeing Ruby, Rails, and Go mixed together.

3v0k4 commented 7 months ago

@kevinmcconnell please let me know if there's anything I can do to help here 🙏

kevinmcconnell commented 7 months ago

Hi @3v0k4, thanks for handling this! (And sorry for my slow response).

I think since Thruster is still young, and pre-1.0, we should drop support for the old env var, rather than support it as a deprecated option. That would simplify the change, and the tests. (Although I appreciate you taking the time to add that!). At this stage of the project I think it's better to arrive at the right API, rather than accrue the baggage of the decisions that we went through to get there, if you see what I mean. It's a breaking change, but it's a very easy one to accommodate. And we can make sure to call it out clearly in the changelog.

Does that sound good to you?

3v0k4 commented 7 months ago

Does that sound good to you?

Sounds great. I'm taking care of it.

3v0k4 commented 7 months ago

@kevinmcconnell updated the code.

kevinmcconnell commented 7 months ago

That's perfect, thanks again @3v0k4!

3v0k4 commented 7 months ago

@kevinmcconnell Thank you!

This was my first Go OSS contribution and it's an honor to have done it paying tribute to Ruby/Rails.

Looking forward for more opportunities to contribute to your work 🙂