senecajs / seneca-web

Http route mapping for Seneca microservices.
MIT License
76 stars 44 forks source link

Break out adapters into sub-modules #100

Closed tswaters closed 8 years ago

tswaters commented 8 years ago

Fixes #94

While this is deprecated, npm might complain about peer dependencies not being met. I'm not entirely convinced this is a valid warning as it is being raised in the peer dependency itself... this only ever shows up if you clone seneca-web and install modules (i.e. during development)

With this PR, if someone installs seneca-web, it will bring down the other packages still and if they are referenced by string when loading via setServer or initialization, it will emit a deprecation warning. The log adapter can be referenced by not providing an adapter.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+4.3%) to 91.76% when pulling 8a0b5b6054337a39226ed7681c63eb9946ec3171 on tswaters:issue-94 into cd2c3d7ec9fe570a93b823ece1fb0ac38481c416 on senecajs:master.

mcdonnelldean commented 8 years ago

I think if we are going to take these out we should do so fully and not auto require the modules; it causes too many weird edge cases. Can you make that change here.

tswaters commented 8 years ago

My concern here is that once this is published people who use the string variant, which is pretty much everyone, will have their app break once they update.

I think it's safer for the ecosystem to deprecate first, people will notice the warning and can have a chance to using the adapters directly once a major version rolls around, remove the dependencies.

mcdonnelldean commented 8 years ago

There are already breaking changes or at least enough to warrant the next release being major. It was a mistake to add the string functionality. It should be aggressively corrected.

Bear in mind the version that had the string support is only the last version. Version before that had a different middleware based way to load stuff.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+4.0%) to 91.538% when pulling f4a8ba636ccc63eaba152f274a7f8653dca4b7fb on tswaters:issue-94 into 1936b8fd219cde81c9cbd3a7fdafa6776e899b28 on senecajs:master.