atais / ng2-eonasdan-datetimepicker

A wrapper directive around the Eonasdan Datepicker v4 component.
MIT License
34 stars 23 forks source link

new structure & test #41

Closed fetrarij closed 6 years ago

atais commented 6 years ago

The tests look fine! Good work on demo running on code from /src/ path of this lib!

But, wow, the structure looks extremely complicated!

I loved that idea of split examples folders because that way you could simply copy such folder and have a working angular.cli or webpack application. Now when it is all embedded into one, its really hard to tell what is still a part of the lib, what is a test and what is a webpack demo or angular.cli conf file.

Was it not possible to simply make a link to app folder` of the other demo classes (simple, reactive, linked datetimepickers)? Or somehow make it separate and link it to both demos?

You did remove 5000 lines of code, which is great, but now I don't think any beginner could use those examples anymore. To be honest I am not sure if I could! :D I hope you could approach it in some other way.

Maybe let's not do everything in one go. Maybe you could 1st find a suitable way for #40 and later for #39, or the other way round... what do you think?

fetrarij commented 6 years ago

@atais it's not complicated as you think. First, to have test working, we can use an webpack structure like ngx-chips structure or angular-cli like ngx-bootstrap or ngx-loading-bar structures. Both are already in our examples, so we can choose one, I choosed the angular-cli so what I did is: moving the angular-cli to the main folder, change its configuration to use source in demo/src. So now in main project, to have test just: ng test , with this structure, we could t now autobuild the gh-pages (which the demo is) with ng build .

Having a structure for webpack here is IMO to test if the project still work with webpack. In this case, I already cleaned up to have the smallest and clearest. The only files that webpack needs here are: webpack folder and webpack.config.json . If you remove theses files, our project will looks like above I mentionned, so I think having it to test is a good idea.

For developer who want to test our demo, just clone,npm install, andnpm run start(for angular cli, or just ng serve) or npm run webpack:start ( for webpack), sources are in demo/src.

Now for beginner who want to use the project, they already should know how to create an angular project (by angular cli ng new or by using webpack) then they just need to follow the readme, all instruction are there, then ater having this working, they can get/use/copy the source in demo/src if they want.

atais commented 6 years ago

Actually seems simple after some explanation, sorry I am not so up to date with Angular2+ projects.

So the last question would be: do you think it is possible to run tests on demos served with webpack? like if we could have:

npm run test:cli
npm run test:webpack

and cli, would do what test does now, but webpack would test if our app is working with webpack.

In that case, when I turn on CI on travis, we could test all our configs and code. What do you think?

fetrarij commented 6 years ago

@atais I dont think it's needed, one test environment suffice, in my opinion , and under the hood angular cli uses Webpack.

atais commented 6 years ago

Sorry I read your comment on weekend but did not respond :) Well, lets go for it then.

fetrarij commented 6 years ago

@atais is it good for you? how about travis ci ? do you want me to do some update?