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

date time improvements #452

Merged
merged 6 commits into from Feb 13, 2017
Merged

Conversation

heiglandreas
Copy link
Contributor

This PR adds some more datetime-improvements by moving the isOpen-checks all to the CallForProposal-class. For that there's a new service callforproposal registered in the app.

Follows #450 and #451.

Copy link
Contributor

@pmeth pmeth left a comment

Choose a reason for hiding this comment

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

looks good, except for a few code sniffer errors and an unused variable. Also, I made a small change in master to remove another code sniffer error, so please rebase before pushing.

@@ -2,7 +2,9 @@

namespace OpenCFP\Domain;

use DateTime;
use DateTimeInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

please reorder your use statements in alphabetical order to prevent triggering the Code Sniffer error ordered_imports in Travis.

@@ -3,6 +3,7 @@
namespace OpenCFP\Test;

use OpenCFP\Application;
use OpenCFP\Domain\CallForProposal;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is unused. did you forget to add something? if not, please remove it to prevent Travis from triggering the Code Sniffer error no_unused_imports.

@@ -38,7 +38,7 @@ public function register(Application $app)
if ($enddate->format('H:i:s') == '00:00:00') {
Copy link
Contributor

Choose a reason for hiding this comment

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

the $enddate variable is no longer used and should be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to merge this into the Silex 2.0 branch and clean up any of the CS errors

@chartjes
Copy link
Contributor

Ugh, @heiglandreas could you redo this PR against the new master?

This commit adds the logic to set the end of the day when only a date is
provided to the CallForProposal-class
This commit moves more checks whether a CfP is open to the
CallForProposal-class. For that the class is registered as a service to
be easily callable.
These changes where necessary to adapt to the new master. It's a rebase
with solved conflicts
This commit introduces changes required by CS-Fixer
@heiglandreas
Copy link
Contributor Author

heiglandreas commented Feb 13, 2017

@chartjes: Rebases and solved conflicts. All tests are green on my machine 😉.

The statement was altered to be able to use sqlite in development. As
sqlite doesn't understand the ```database()```-command I needed to use
something else…

This commit reintroduces the old query to work on travis
@chartjes
Copy link
Contributor

Thanks @heiglandreas

@chartjes chartjes merged commit b5d2250 into opencfp:master Feb 13, 2017
@heiglandreas
Copy link
Contributor Author

I removed that getTalkForm during the rebase…

@chartjes
Copy link
Contributor

@heiglandreas Yeah, I saw but somehow I messed it up. I pushed code again and tests pass now

@heiglandreas heiglandreas deleted the hotfix/DateTime branch February 13, 2017 15:16
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

3 participants