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

Moved Gridfield #4323

Closed
wants to merge 1 commit into from
Closed

Moved Gridfield #4323

wants to merge 1 commit into from

Conversation

assertchris
Copy link
Contributor

:trollface:

@dhensby
Copy link
Contributor

dhensby commented Jun 20, 2015

Well, this will make merging up fun.

Do we want to wait until 3.2 is released and merged up to do this? @sminnee

@willmorgan
Copy link
Contributor

I think this could go in to 3.2 as it's more of a directory structure patch.

@silverstripe/core-team - any thoughts?

@dhensby
Copy link
Contributor

dhensby commented Jun 26, 2015

I don't mind where this goes. I guess 3.2 would be best

@willmorgan willmorgan added this to the 3.2 beta 2 milestone Jun 26, 2015
@willmorgan
Copy link
Contributor

Gonna say some more @silverstripe/core-team should weigh in on this, but I am strongly for 3.2.

@chillu
Copy link
Member

chillu commented Jun 26, 2015

What's the point of this? GridField extends FormField, like all others in this folder. I don't think merging up would be an issue (git knows about path changes), so not too fussed about which branch it goes into. But I don't follow the logic for this change in the first place.

@dhensby
Copy link
Contributor

dhensby commented Jun 27, 2015

It's in perpetration for namespacing.

The idea is to break the core components into namespaces and the namespaces should hold the minimum required.

Therefore gridfield won't be part of the core forms component, it would be its own that had a dependency on form

@chillu
Copy link
Member

chillu commented Jun 27, 2015

But shouldn't we wait with this folder restructure until we actually introduce namespaces? Or move components out of core into their own modules? Just moving one small subset of files around seems like busywork to me.

@tractorcow
Copy link
Contributor

I feel the same as @chillu ; It's a subset of forms, and probably should be in the same parent namespace.

I've already done a pass over the framework code to namespace things (psr4) https://github.com/tractorcow/silverstripe-framework/tree/pulls/3.2/namespaces/src/SilverStripe/Framework/Forms/Fields

I had namespaced GridField directly under SilverStripe/Framework/Forms/Fields, but all the supporting gridfield classes were within the nested SilverStripe/Framework/Forms/Fields/GridField namespace.

My vote would be to move it when we do the namespacing, whatever we decide to put it under, rather than trying to do it as a part of a separate early stage. It doesn't have much point to move it at this stage.

@assertchris assertchris closed this Jul 2, 2015
@assertchris assertchris deleted the move-gridfield branch September 7, 2015 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants