search

tmatilai / vagrant-proxyconf

Vagrant plugin that configures the virtual machine to use proxies
MIT License
531 stars 74 forks source link

add initctl reload-configuration for old ubuntu / debian without systemctl / Update docker_proxy_conf.rb #228

Closed MaxPeal closed 3 years ago

MaxPeal commented 4 years ago

add initctl reload-configuration for old ubuntu / debian without systemctl

MaxPeal commented 4 years ago

with my tests its work, but im stuck with with 3 failures at bundle exec rspec

comments are welcome

codylane commented 4 years ago

hi @MaxPeal - Wow, this is awesome, thank you so much for taking the time to address this as well as write some tests. I'll gladly share some thoughts on how to get the tests to pass when I have some extra time to do so.

I did look at the travis failures and it looks like all we need to do is adjust some of the stubs. But as you probably already have seen, it's a bit of a mess and can be a little confusing. That said, give me a few days so that I can collect my thoughts, continue testing and i'll provide an example of how to get those tests to pass.

I appreciate your time and I'm grateful for the time you have spent preparing the pull request and linking the issues. I'll be in touch in the next few days.

MaxPeal commented 4 years ago

@codylane sounds good,

can you please make a pre-release with this RP or a part of it? it can be installed it with

vagrant plugin install --plugin-source https://rubygems.org/ --plugin-prerelease vagrant-proxyconf

so we get it good testet with more user.

codylane commented 4 years ago

@codylane sounds good,

can you please make a pre-release with this RP or a part of it? it can be installed it with

vagrant plugin install --plugin-source https://rubygems.org/ --plugin-prerelease vagrant-proxyconf

so we get it good testet with more user.

Hi @MaxPeal - I can totally appreciate your sense of urgency and for wanting to get this released even as a pre-release. However, if the tests are failing I have a hard limitation that no failing tests can be part of any release. That said, there is a lot of clean up that needs to happen in order to support this change and it's not easy and it might seem. I appreciate your time and your responses but as a maintainer of this plugin I need to also make sure that any change introduced to this library does not cause regressions for existing behavior.

The regression testing is the biggest challenge for this plugin and it's very much a manual approach for me. It can take me days/weeks sometimes to ensure the code being released does not introduce a different bug. Please hang tight, I promise to get to this when I have more time but keep in mind this an open source and I am not being paid to manage this with 24x7 turnaround.

I take bugs seriously, but this isn't one that I've considered an high enough priority especially since 14.04 is a dumpster fire and is EOL. I've spent a fair amount of time in the past supporting 14.04 and all the craziness it did between trying to manage services through upstart initctl rc-update.d and service! OMG!

I realize there are still environments or politics that require the use of old EOL tech but it's a lot harder to maintain code that is future and backwards compatible to all versions. At some point, we will need to take a stance to say we can only support current stable and future.... However, that is not officially stated yet so I'm open to adding this in, it's just going to take some time as I cannot accept the PR as it stands today. It wouldn't make this plugin future proof, it would make it harder to maintain.

codylane commented 3 years ago

As discussed in a previous thread, I cannot accept this pull request as is since the tests are failing and it introduces new code paths for EOL versions of linux.

I apologize but I don't have the time to try and retro fit the current state of the plugin to support an older version of linux. I do appreciate the time you spent in this and I apologize that I couldn't be convinced this was worth the refactor.