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 components up a level? #13

Closed
dylans opened this issue Feb 1, 2017 · 8 comments
Closed

Move components up a level? #13

dylans opened this issue Feb 1, 2017 · 8 comments
Assignees
Milestone

Comments

@dylans
Copy link
Member

dylans commented Feb 1, 2017

Now that widgets have been split from widget-core, I think we should move the contents of /components up a level into src. Or do we really want things to be @dojo/widgets/components/createFoo ?

@dylans dylans added this to the 2017.02 milestone Feb 1, 2017
@agubler
Copy link
Member

agubler commented Feb 1, 2017

I think you absolutely right (i noticed it the other day but forgot), I also think we should drop components directory.

@tomdye
Copy link
Member

tomdye commented Feb 1, 2017

Agreed 👍

@rishson
Copy link
Contributor

rishson commented Feb 1, 2017

Can we also move example usage into the widget's tests dir please?
This removes the parallel dirs, and keeps everything together.
I propose someWidget/tests/example or someWidget/tests/functional as the location for the example usage / test harness.
Also happy just with top level: someWidget/example

@agubler
Copy link
Member

agubler commented Feb 1, 2017

@rishson I don't think that the examples should not be in the test directory, personally I think they should live along side the widgets. Such src/dialog/createDialog.ts would have src/dialog/example(s)/dialog.ts.

@rishson
Copy link
Contributor

rishson commented Feb 3, 2017

So were agreed on:

src/dialog/example
src/dialog/styles
src/dialog/createDialog.ts
tests/unit/createDialog.ts

@msssk
Copy link
Contributor

msssk commented Feb 3, 2017

I'm in favor of src only containing code relevant to the repo's core purpose, which for dojo-widgets is providing widgets. tests are not the core purpose and are rightly placed as a sibling of src, I feel like examples (which I slightly favor renaming to demos) should also be in a sibling folder of src.

This could also simplify builds - if we want to build a distribution for consumption as a dependency, the consumer just wants the widgets, not tests or examples. Compile and build everything in src and we're done, no need to specify exclusions.

Basically, everything that should be provided to a dependency consumer should go in src, anything not relevant to dependency consumers should be kept out of src.

demos/dialog/index.html
demos/dialog/index.ts
src/dialog/styles/dialog.css
src/dialog/createDialog.ts
tests/unit/createDialog.ts

@agubler
Copy link
Member

agubler commented Feb 4, 2017

I understand where you're coming from, but I would even prefer to have the tests in src (like jest), because then everything related is together. Don't forget just because it is src doesn't mean we'll publish it (we won't) so consumer of the package won't receive the examples.

@dylans
Copy link
Member Author

dylans commented Feb 4, 2017

@agubler would this approach just be for @dojo/widgets? I guess I would argue that examples and test probably should be there for someone learning Dojo 2, but not for a production-ready distribution?

Regardless, I think this approach would get cumbersome for something like @dojo/core, but I can see the logic for widgets because the idea is to think of each widget as independent (so each widget could live in a separate repo, but we don't want the overhead of ever widget being in a separate repo).

The only potential negative I see with this is that it will make the Intern config more complex (with regards to how you describe paths for testing and what to exclude from testing, but not overly arduous.

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

No branches or pull requests

6 participants