Skip to content

Integrate Test Workspaces/Crates #637

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

Closed
JP-Ellis opened this issue May 13, 2019 · 7 comments
Closed

Integrate Test Workspaces/Crates #637

JP-Ellis opened this issue May 13, 2019 · 7 comments

Comments

@JP-Ellis
Copy link
Contributor

While working on another pull requests, I noticed that the workspaces seem to be often neglected as people don't look into them, and it made me question why we have them in the first place.

In particular, three of them contain only tests, which makes me wonder whether they should really be in a workspace of their own, or whether these tests should be migrated into the main tests directory. The particular workspaces in question are:

  • blas-tests which should be fine to have in the tests directory and gated behind the blas feature
  • numeric-tests
  • serialization-tests

I'm happy to work on a pull requests to remove these three workspaces and move the top-level tests directory.

@JP-Ellis JP-Ellis changed the title Remove workspace Remove Test Workspace May 13, 2019
@max-sixty
Copy link
Contributor

I was just about to lodge the same issue. Any thoughts?

@max-sixty
Copy link
Contributor

(I think strictly this should be "integrate test crates" but the meaning is clear)

@LukeMathWalker
Copy link
Member

@jturner314 left comments about this in #628 - we could probably start from there to sort this out @max-sixty

@JP-Ellis JP-Ellis changed the title Remove Test Workspace Integrate Test Workspaces/Crates Sep 3, 2019
@bluss
Copy link
Member

bluss commented Sep 4, 2019

They should not be in the main tests - they are compiled separately for various reasons, for example blas-tests to use its own blas-enabling settings and to not slow down the regular development cycle ( you don't have to compile it by default if it's not in the main tests).

Please respect the contributors' time and don't bloat the default dev dependencies too much.

@JP-Ellis
Copy link
Contributor Author

JP-Ellis commented Sep 4, 2019

Apologies if you found this issue disrespectful.

If the main concern is how long it takes to build the various dev dependencies, then fair enough, there isn't much that can be done at this stage. Feel free to close this issue as "won't fix / intended".

@bluss
Copy link
Member

bluss commented Sep 4, 2019

This is forceful but it's about the code changes, it's not personal. Respecting the contributors time is a design constraint, like, let's make it a joy to work on and one of the goals we can have to do that, is to limit compile time needed to work on changes.

I am of two minds - it's good that ndarray is developing a lot outside of my comfort zone. But I still see that there is nobody to really provide a real vision if I don't - it needs more than stewardship. I would be a type of contributor that leaves if it gets too slow to work on the crate.

@JP-Ellis
Copy link
Contributor Author

JP-Ellis commented Sep 4, 2019

Like I said, I completely understand and you are free to close this issue as "won't fix / intended". I agree that if it takes too long to compile ndarray, or if the test suite takes too long to build every time, then people might not contribute as much. I opened this issue not knowing the reason behind the (seemingly arbitrary) separation of certain tests into workspaces.

But I still see that there is nobody to really provide a real vision if I don't - it needs more than stewardship.

Have you considered opening a new issue asking contributors what can/should be done about this? (I haven't searched for this, so it's possible this has been done).

@bluss bluss closed this as completed Sep 12, 2019
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

4 participants