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

Clean-up #5

Closed
nilshoerrmann opened this issue Jul 23, 2014 · 10 comments
Closed

Clean-up #5

nilshoerrmann opened this issue Jul 23, 2014 · 10 comments

Comments

@nilshoerrmann
Copy link

Old code from SBL, things related to backwards-compatibility etc. should be removed.

@brendo: Would you mind having a look, if there are things we can simplify? Thanks!

@brendo
Copy link
Member

brendo commented Aug 9, 2014

Can do!

Just a question though, are you hoping to break the dependancy on the SBL field? There are a number of functions in this field that could be removed if we extended the SBL instead (filtering, resolving values etc.)

@nilshoerrmann
Copy link
Author

The idea was that this extension will replace SBL in Symphony 2.5 in order to get consistent names across association extensions (remember our chat?). So SBL will continue to work for Symphony versions up to 2.4 and Association field will support Symphony 2.5+ only. Our idea was to deprecate SBL on 2.5 providing a migration script. But Association field will not have to support older Symphony or PHP versions.

@nilshoerrmann
Copy link
Author

So to make this more precise:

  • This extension should make use of the latest Symphony methods as if it was a new extension.
  • This extension does not have to support older Symphony versions.
  • There are historic code fragments from the Symphony 2.0beta times that should be reviewed.

So this is a call for a general SBL clean-up, now rebranded Association field.

@nilshoerrmann
Copy link
Author

Hey Brendan, have you found the time to take a look at this?

@brendo
Copy link
Member

brendo commented Sep 16, 2014

Not at all sorry.

On Tue, Sep 16, 2014 at 6:21 PM, Nils Hörrmann [email protected]
wrote:

Hey Brendan, have you found the time to take a look at this?


Reply to this email directly or view it on GitHub
#5 (comment)
.

@nilshoerrmann
Copy link
Author

No problem, just wanted to check.

@nilshoerrmann
Copy link
Author

It would be great, if we could tackle this before the release of Symphony 2.6 – at least all bug fixes to SBL should be merged in.

@twiro
Copy link
Member

twiro commented Jul 21, 2015

It would be great, if we could tackle this before the release of Symphony 2.6 – at least all bug fixes to SBL should be merged in.

BTW: I created an "SBL"-Branch of the extension, that backports all additions to SBL (that haven't been included in this repo yet) and follows the versioning/naming-scheme of SBL:

I submitted the commits one by one and referenced to the original ones:

As long as SBL and Association Field coexist (which in my opinion is not a good solution in the long run) it might be helpful to couple updates & versions of both extensions (except for enhanced features of the association field).

@nilshoerrmann
Copy link
Author

Cool.

I'm not sure how the team is going to handle SBL and AF in the future – or if it's going to be handled at all (well the idea one and a half year ago was to deprecate the first and move on with the second extension). Maybe @brendo can chime in here.

Version 1.0.0 is just a summary of the open changes and fixes.

@twiro
Copy link
Member

twiro commented Jul 21, 2015

Moved the discussion about my proposal to a separate issue:

I'm not sure how the team is going to handle SBL and AF in the future – or if it's going to be handled at all (well the idea one and a half year ago was to deprecate the first and move on with the second extension).

Well... while it's surely not perfect that a clear cut was missed right at the start and association field wasn't treated as the successor/replacement for SBL I think as of now it wouldn't make much sense to change this before the next major relase of Symphony.

And I somehow feel that it's partly my fault as I insisted on changing the name of the extension ;)

Will probably open a separate issue for generally discussing this topic the next days.

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

3 participants