Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

PostgreSQL support. Configure options array. Issue #10 resurrected: custom jobs table name. #23

Merged
merged 2 commits into from
Feb 4, 2014

Conversation

jfloff
Copy link
Contributor

@jfloff jfloff commented Feb 14, 2013

Decided to implement couple of stuff, here's a breakdown:

  • PostgreSQL support: Generalize some MySQL specific code to work on both db engines. Created a new .sql file to create a PostgreSQL jobs table and a specific test file.
  • Implemented configure to take options array: Was on the todo list of the repository. At the same time, revamped the connection parameters to be more generic (no need for specific MySQL type of connection parameters anymore).
  • Resurrected issue Let me know if this solution works for you. I am happy to go another route if you like. #10: Since I was at it, thought this one was worth the effort ;)
  • I also added a .gitignore file .

SQLite support is something that I though could be useful, but early investigations lead me to delay it, due to possible heavy changes in SQL code (bulk INSERT's mainly).

EDIT:
If you want me to rebase to one commit please say so.
Typos.

@josegonzalez
Copy link
Contributor

I'm worried about that bc change, which could end up breaking existing installations - such as our own! - so I'm not going to merge this in without further explanation.

Apologies for taking so much time to get to this.

@jfloff
Copy link
Contributor Author

jfloff commented May 15, 2013

Really sorry I didn't document that well.

Hope my explanation brings some light into why I went that route. I can gladly try to change this to be BC.

No apologies needed :)

@josegonzalez
Copy link
Contributor

I'd rather revert any bc changes and then have a separate commit that brings in postgres support.

You may need to rebase your commit as I've recently updated the tests and some retry code was added.

@jfloff
Copy link
Contributor Author

jfloff commented May 16, 2013

I'll do that then. Will do a first commit for everything except postgres. Then will commit postgres support.

Kudos

@jfloff
Copy link
Contributor Author

jfloff commented May 16, 2013

Heres the commit without postgres support. Hope that alright. I'll now take a look at postgres support without breaking BC.

public static function configure(array $options, $jobsTable = 'jobs') {
self::$jobsTable = $jobsTable;

if(!isset($options['driver']))
Copy link
Contributor

Choose a reason for hiding this comment

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

use spaces before the first parens and use {} for codeblocks.

josegonzalez added a commit that referenced this pull request Feb 4, 2014
PostgreSQL support. Configure options array. Issue #10 resurrected: custom jobs table name.
@josegonzalez josegonzalez merged commit 35c26e3 into seatgeek:master Feb 4, 2014
@josegonzalez
Copy link
Contributor

Thanks for the pr!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants