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

[WIP] 1.0 release #183

Closed
wants to merge 54 commits into from
Closed

[WIP] 1.0 release #183

wants to merge 54 commits into from

Conversation

K-Phoen
Copy link
Collaborator

@K-Phoen K-Phoen commented Dec 24, 2013

My PR integrating Propel became quite big as I kept pushing stuff not related to Propel, so I decided to reopen the PR under a new name.

I added a few new features, tried to solve a few problems and cleaned the code/tests. I think this could be the right moment to release a 1.0.0 version.

Here is a quick summary of what should be done for this new version:

Does anyone see other things to do?

{
return $e->getDocument();
/* @var $event \Doctrine\Common\EventArgs */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed

@Baachi
Copy link
Contributor

Baachi commented Dec 24, 2013

Really cool 👍

@K-Phoen
Copy link
Collaborator Author

K-Phoen commented Dec 25, 2013

Big changes with the commit 7c71c18. The event listeners are now registered per action (inject, upload, ...) and per mapping. This allows us to simplify several parts of the code (the storage and the injector part for instance).

@K-Phoen
Copy link
Collaborator Author

K-Phoen commented Dec 26, 2013

I also would like to have your opinion on #185

@ftassi
Copy link
Collaborator

ftassi commented Dec 29, 2013

@K-Phoen you're doing a lot of work, I want to thank you, I really appreciate that. I have not reviewed any code of this right now so I don't have any feedback about the code.

I'd like to give you my thoughts about the method. I think that our work with this bundle could be a lot easier if you'd try to split down your work in smaller chunk of code, so you can make smaller PR that we can marge faster and easier. Don't be obsessed by the "1.0" version, I guess is not the version number that matter, the features are.

As far as I can see there is a lot of stuff here that could be merged (fixes, xml meta driver, PropertyMapping usage refactoring and so no) that are not actually on master and not shipped to users (that can give us feedback) because they are included in this monolithic PR.

Smaller PR means faster merge, release and feedback from users. As usual when talking about feedback, the faster the better.

If you need some kind of road map (which I can agree with) use issues for that. We could create(I think that @dustin10 which have admin rights on the repo need to do this) a milestone and assign issues to that milestone to track how the work is going.

What do you think ?

@K-Phoen
Copy link
Collaborator Author

K-Phoen commented Jan 1, 2014

On Sun, Dec 29, 2013 at 11:51 PM, Francesco Tassi
[email protected]:

@K-Phoen https://github.com/K-Phoen you're doing a lot of work, I want
to thank you, I really appreciate that. I have not reviewed any code of
this right now so I don't have any feedback about the code.

I'd like to give you my thoughts about the method. I think that our work
with this bundle could be a lot easier if you'd try to split down your work
in smaller chunk of code, so you can make smaller PR that we can marge
faster and easier. Don't be obsessed by the "1.0" version, I guess is not
the version number that matter, the features are.

As far as I can see there is a lot of stuff here that could be merged
(fixes, xml meta driver, PropertyMapping usage refactoring and so no) that
are not actually on master and not shipped to users (that can give us
feedback) because they are included in this monolithic PR.

I usually work like this but this time I mostly committed stuff while I was
reading the actual code base and I didn't take the time to split the
commits in terms of functionalities. Sorry for the big mess...

I will split this huge PR in smaller ones to ease the review process, don't
worry ;)

Smaller PR means faster merge, release and feedback from users. As usual
when talking about feedback, the faster the better.

If you need some kind of road map (which I can agree with) use issues for
that. We could create(I think that @dustin10 https://github.com/dustin10which have admin rights on the repo need to do this) a milestone and assign
issues to that milestone to track how the work is going.

What do you think ?

I think we already can create milestones. It's indeed something we could
use.

@K-Phoen K-Phoen mentioned this pull request Jan 18, 2014
@Baachi Baachi mentioned this pull request Mar 26, 2014
3 tasks
@benglass
Copy link
Contributor

@K-Phoen we'd like to be able to use the phpcr support I added and also the multi-driver support you and I discussed. Do you have a sense of what is outstanding to get 1.0 wrapped/released? Anything I can assist with?

@K-Phoen
Copy link
Collaborator Author

K-Phoen commented Jun 11, 2014

Here is what I'd like to do before the 1.0 release:

  • PHPCR support
  • multi-driver support
  • bugfix
  • check that the bundle works with GridFS and MogileFS (might induce a refactoring)
  • reorganize the doc
  • refactor event listeners (as I did in the 1.0 branch and as suggested in the PR [WIP] Refactor event system #106). I'd like to avoid this kind of nasty workaround. Ideally, uploads should work out of the box with listeners but we should be able to disable them (or some of them) and trigger the upload/other operations manually.

Non-blocking issues:

  • code cleaning
  • tests cleaning (our testsuite isn't really good right now...)
  • Propel2 support

I'll create some issues and a milestone for the 1.0 release.

@K-Phoen K-Phoen closed this Oct 8, 2014
@K-Phoen K-Phoen deleted the 1.0 branch October 8, 2014 08:40
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.

How can I add subfolder when using gaufrette
4 participants