Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move evil-test-helpers.el to its own repo? #1882

Open
jcs090218 opened this issue Apr 9, 2024 · 6 comments
Open

Move evil-test-helpers.el to its own repo? #1882

jcs090218 opened this issue Apr 9, 2024 · 6 comments

Comments

@jcs090218
Copy link
Contributor

Opting for a single package per repository appears to be a preferable option, as indicated in melpa/melpa#8172. This approach also streamlines the process of conducting CI tests and enhances clarity regarding the association between packages. 🤔

@tomdl89
Copy link
Member

tomdl89 commented Apr 9, 2024

@jcs090218 before I answer specifically to this issue, I should preface by saying I'm not such a big fan of purely "correctness" changes to established projects like evil. But I'm happy to be convinced when it's more than just correctness.

Specifically to this issue, I read melpa/melpa#8172 and I'm not sure the presence of evil-test-helpers.el in evil's repo is particularly relevant. That policy is about separate packages being in all-in-one repos, but I think it's too much of a stretch to say evil-test-helpers.el is a separate package. I'm not aware of it being required by users or other packages in a scenario where at least some part of the rest of evil isn't required.

The sell that it "also streamlines the process of conducting CI tests" sounds alluring, but what do you actually mean? Can you be more concrete? When I think about it, I'm not sure I see much in the way of practical streamlining. Maybe I'm missing something.
And on your last point, I get the basic idea that splitting things out enhances clarity, but like I said, I don't really think they're separate packages, so I'm not sure why we need to "enhance clarity" between them..?

@jcs090218
Copy link
Contributor Author

The maintainer(s) are not interested. Closing this issue then. :)

@jcs090218 jcs090218 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
@tomdl89
Copy link
Member

tomdl89 commented Apr 9, 2024

I should reiterate that I welcome counter-arguments to my points. I'm not immediately "interested" insofar as wanting to make the change without further discussion. But I am "interested" insofar as being curious about arguments I hadn't considered. I hope you don't feel I've been overly dismissive.

@jcs090218
Copy link
Contributor Author

jcs090218 commented Apr 9, 2024

I should reiterate that I welcome counter-arguments to my points. I'm not immediately "interested" insofar as wanting to make the change without further discussion. But I am "interested" insofar as being curious about arguments I hadn't considered. I hope you don't feel I've been overly dismissive.

Sorry, I shouldn't have gone to the conclusion that quickly. I apologize. I will elaborate more below.

The sell that it "also streamlines the process of conducting CI tests" sounds alluring, but what do you actually mean? Can you be more concrete? When I think about it, I'm not sure I see much in the way of practical streamlining. Maybe I'm missing something.

I was planning to contribute to this project by first replacing Cask with Eask since Cask has been deprecated for a long time. I would then clean up the Makefile and add tests for macOS and Windows. Similar to the PR emacs-evil/evil-collection#804.

And on your last point, I get the basic idea that splitting things out enhances clarity, but like I said, I don't really think they're separate packages, so I'm not sure why we need to "enhance clarity" between them..?

I was a bit confused by this since evil-test-helper itself has its own recipe on MELPA, so I assumed they are two separate packages... no? 🤔

Other than that, having tests in the test folder would help clarify what package files and test files are. Should evil-tests.el be moved into the test folder?

@jcs090218 jcs090218 reopened this Apr 9, 2024
@tomdl89
Copy link
Member

tomdl89 commented Apr 10, 2024

Thanks @jcs090218 . I read #846 and agree with @wasamasa 's comment:

I'd be in for this change, as long as evil-tests-helpers.el remains in this repository so that Evil itself doesn't need to do anything else than (require 'evil-tests-helpers) in its test file.

So it looks like they decided to make evil-test-helpers its own package, but they let the recipe point to evil's repo, so they are always in sync. As they concluded, I think keeping it in the evil repo is the right thing to do. I'm still not aware of any arguments to do otherwise.

With regard to moving evil-tests.el and evil-test-helpers.el to a test folder, I'm not strongly opposed to it, but I don't think it affects clarity that much either way. If you want to make a PR, I would probably merge it.


I was planning to contribute to this project by first replacing Cask with Eask since Cask has been deprecated for a long time. I would then clean up the Makefile and add tests for macOS and Windows. Similar to the PR emacs-evil/evil-collection#804.

I don't think we actually use Cask, but I do in general like the idea of us moving to something like Eask. I introduced Eldev to evil-cleverparens and I do like it. Evil's current approach is a bit too "hand-rolled" for my liking. PR welcome on this.

@jcs090218
Copy link
Contributor Author

So it looks like they decided to make evil-test-helpers its own package, but they let the recipe point to evil's repo, so they are always in sync. As they concluded, I think keeping it in the evil repo is the right thing to do. I'm still not aware of any arguments to do otherwise.

Thanks for clarifying this! :)

With regard to moving evil-tests.el and evil-test-helpers.el to a test folder, I'm not strongly opposed to it, but I don't think it affects clarity that much either way. If you want to make a PR, I would probably merge it.

👍

I don't think we actually use Cask, but I do in general like the idea of us moving to something like Eask. I introduced Eldev to evil-cleverparens and I do like it. Evil's current approach is a bit too "hand-rolled" for my liking. PR welcome on this.

Oh, sorry. I saw the Cask file, so I assumed this package was using Cask. 😓

Eldev is another good choice, but Windows support is a second-class citizen. As the author of Eask, I treat all modern OSs as first-class citizens. 🤔 BTW, evil-cleverparens looks very interesting! 😋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants