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

50 shades of Refactoring on Dates, Added Doctrine_Mongo_Date_Filter a… #142

Closed
wants to merge 1 commit into from

Conversation

joshlopes
Copy link
Contributor

50 shades of Refactoring on Dates, Added Doctrine_Mongo_Date_Filter and Doctrine_Mongo_DateRange_Filter and Doctrine_Mongo_DateTime and Doctrine_Mongo_DateTimeRange_Filter

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #58
License no
Doc PR no

@soullivaneuh
Copy link
Member

As a new feature, could you please update documentation?

@joshlopes
Copy link
Contributor Author

@soullivaneuh added, also enabled the filterGuesser which was disabled for those filters, Added a test for the guesser and testing those which i created and the documentation. looks better now .. :)

@soullivaneuh
Copy link
Member

Ping @rande for review.

@joshlopes When review is terminated, could you please squash your commit? 👍

@joshlopes
Copy link
Contributor Author

@soullivaneuh sure thing -- just let me know after review also there is other PRs that needs to be reviewed and issues to be closed

.. and that close was a mistake :)

@joshlopes joshlopes closed this Jun 11, 2015
@joshlopes joshlopes reopened this Jun 11, 2015
@@ -1,17 +1,8 @@
<?php

/*
* This file is part of the Sonata package.
Copy link
Member

Choose a reason for hiding this comment

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

You need to keep headers files.

Copy link
Member

Choose a reason for hiding this comment

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

headers are here to protect authors and contributors, this is very important.

@soullivaneuh
Copy link
Member

Please rebase your PR to get new Travis checks, and then fix CS.

@joshlopes
Copy link
Contributor Author

@soullivaneuh the fatals are on the vendores folder -- PHP Fatal error: Interface 'Symfony\Component\Security\Acl\Model\DomainObjectInterface' not found in /home/travis/build/sonata-project/SonataDoctrineMongoDBAdminBundle/vendor/sonata-project/admin-bundle/Admin/Admin.php

on the Admin Project, as for the CS i do not know why it fails and passes to others. make no sense - i am missing something ?

@soullivaneuh
Copy link
Member

Please rebase your PR first to see if you are still getting this error.

For CS, juste run make cs on you project to get the fix. Make sure to have the last version of php-cs-fixer with composer update.

…nd Doctrine_Mongo_DateRange_Filter and Doctrine_Mongo_DateTime and Doctrine_Mongo_DateTimeRange_Filter
@joshlopes
Copy link
Contributor Author

Rebased , commits squashed , make cs done -- passed, make test done -- passed.

@soullivaneuh
Copy link
Member

Thanks. I let @rande or @dunglas do the last review and merge.

@SonataCI
Copy link
Collaborator

SonataCI commented Jun 4, 2016

Could you please rebase your PR and fix merge conflicts?

@soullivaneuh
Copy link
Member

According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning.

If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch.

So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one).

Regards.

@webdevilopers
Copy link

@soullivaneuh @joshlopes Do I understand this correctly - the filteres featured here never made it into any release?

@joshlopes
Copy link
Contributor Author

@webdevilopers I've stopped using mongo many moons ago , there were a few PRs that sadly got stale for very long time and i moved on and many of them never got to seen merged sadly.

@franmomu
Copy link
Member

hey @joshlopes, thanks for your PRs, I'll try soon to take a look at those PR to see if they are still valid.

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.

7 participants