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

Just a little fix for a tree_panel. It just work, but is still bad-documented and not-tested #9

Merged
merged 1 commit into from Apr 22, 2013

Conversation

phgrey
Copy link
Contributor

@phgrey phgrey commented Apr 21, 2013

No description provided.

mxgrn pushed a commit that referenced this pull request Apr 22, 2013
Just a little fix for a tree_panel. It just work, but is still bad-documented and not-tested
@mxgrn mxgrn merged commit 9e08b92 into netzke:master Apr 22, 2013
@mxgrn
Copy link

mxgrn commented Apr 22, 2013

Thanks! I'm merging this in without checking it out myself - afaik, it's been broken for a while.

@phgrey
Copy link
Contributor Author

phgrey commented Apr 24, 2013

Hi!
First - thanks a lot for the netzke. It's realy good. And it saves me
a lot of time.

Another thank for merging this small fix.

I'd like to ask - i've written TreeGridPanel as an extension of
Basepack::Grid.
It has < 100 lines of code and heavy depends on parent (it even uses
it's mixin grid.js)
I'd like to publish it as soon as I'll write tests for this component.
And that's what I'd like to ask You - may I make another pull request
on a netzke-basepack with that component or I should create separate
gem?

Thanks and best regards.
Sergey.

2013/4/22 Max Gorin notifications@github.com

Thanks! I'm merging this in without checking it out myself - afaik, it's
been broken for a while.


Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-16792249
.

@mxgrn
Copy link

mxgrn commented Apr 24, 2013

Sergey,

It'd be simply awesome if you released it as your own gem (having Basepack as dependency). You're free to name it the way you want, but maybe some "brand"-prefix would be handy (something like phgrey-netzke-treepanel) - because @scho, for example, is working on his own version of TreeGrid. Make sure you document it well enough so other people can start using it right away. I'll be happy to list it somewhere on the netzke.org. Really looking forward to what you come up with!

Btw, what are you planning to use for testing? Have you checked out my approach with Mocha + expect.js + RSpec in Netzke Core? Hope it can help.

@mxgrn
Copy link

mxgrn commented Apr 24, 2013

Btw, I totally dig reusing the Basepack code as much as possible. But, please be aware, that the risk here is that your component will be very much dependent on the internals of Basepack::Grid… I hope, with the time we'll work out some way to handle this. If you see where Basepack::Grid code can be refactored to serve you better - I'll be glad to discuss.

Thanks for you work, too!

@phgrey
Copy link
Contributor Author

phgrey commented Apr 24, 2013

Yeah, I'm already trying to use rspec + mocha.js for testing.
Yeah, I think You're right about separate gem. I'm still thinking about
cofee.js runtime dependency.
In the project I'll use several more components (Ext.view etc), will try to
collect them in one gem or several gems If I'll have enough time.

About heavy-dependence - for now I do think to add checking on method
existing in my tests so I'll be able to quick react on changes in
Netzke::Core an Basepack::Grid.

Well, Max, thanks a lot for Your advise and answers. Will contact later
when I'll be ready.
And thanks again for the Netzke!

2013/4/24 Max Gorin notifications@github.com

Btw, I totally dig reusing the Basepack code as much as possible. But,
please be aware, that the risk here is that your component will be very
much dependent on the internals of Basepack::Grid… I hope, with the time
we'll work out some way to handle this. If you see where Basepack::Grid
code can be refactored to serve you better - I'll be glad to discuss.

Thanks for you work, too!


Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-16924164
.

@mxgrn
Copy link

mxgrn commented Apr 24, 2013

About heavy-dependence - for now I do think to add checking on method existing in my tests so I'll be able to quick react on changes in Netzke::Core an Basepack::Grid

I'd say a good (integration) test coverage would be enough.

Especially if you decide to come up with multiple gems (each per (complex) component), we maybe should think about extracting a little Netzke testing micro-framework that can be included into any new netzke gem without writing any code. I already see duplications between Core and Basepack… Will think about it!

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

Successfully merging this pull request may close these issues.

None yet

2 participants