search

rejeep / prodigy.el

Manage external services from within Emacs
GNU General Public License v3.0
550 stars 39 forks source link

Provide scoped test helper #96

Closed rejeep closed 7 years ago

rejeep commented 7 years ago

Simply calling it test-helper conflicts with other packages. See https://github.com/rejeep/prodigy.el/issues/94 for more information.

tarsius commented 7 years ago

Since you won't be able to (require 'prodigy-test-helper), what is the benefit of prefixing the provided feature with the package name without also renaming the file, as opposed to just not providing a feature at all?

rejeep commented 7 years ago

None, but I noticed I provided all the other test files so I figured I would be consistent. That is either scope the test helper or remove all provides. And this was simpler.

tarsius commented 7 years ago

That is either scope the test helper

Which would break ert-runner because it expects the file to be named test-helper.el, right?

or remove all provides.

I think simply living with the inconsistency would be a third and better alternative. I am all for consistency (otherwise I would not have brought up this issue in the first place), but what you have opted for is also inconsistent: now all other files provide the correct feature (and are therefore requireable), while this one provides the "wrong" feature for purely cosmetic/consistency reasons ;-)

My worry is that, because you are the maintainer of ert-runner, someone who uses that could copy what you have done here, but without understanding the consequences and while expecting (require 'PACKAGE-test-helper) to work. Or worse, noticing the issue and then "fixing" it by renaming the file, thus breaking ert-runner.

And you might have to come up with a new behavior for ert-runner. I think it would be nice if ert-runner created and later loaded PACKAGE-test-helper.el (while falling back to test-helper.el for projects that already use that). I am just not sure if ert-runner can always know the value of PACKAGE, so this might not be feasible.