Countries, currencies and regions infrastructure update #1865
Conversation
…than from the database. Allows internationalization of country, region and currency information Eliminates a significant amount of data being read from the database at startup Removes several hundred lines of code from what we have to maintain and also improves speed and lowers memory footprint
|
Items in progress
|
|
@leewillis77 @JustinSainton this is working in my test environment, i'll keep working on it, but it is reviewable in its current state |
| @@ -0,0 +1,193 @@ | |||
| <?php | |||
There was a problem hiding this comment.
@JustinSainton @leewillis77 I would like this file to contain our outward facing procedural api for countries/regions/currency operations. Dev's certainly can go right at the class interface for the WPSC_Countries class, but that class has a fair amount of complexity because of some backwards compatibility requirements.
For most simple operations that a plugin or theme might want to do this file seem to be the API we should promote, in combinatiuon with the WPSC_Country and WPSC_Region objects that the functions within the file return
Use documented API function for countires operation rather than static class method
| * */ | ||
| function wpec_taxes_get_country_information( $columns = false, $where = array(), $order_by = false ) { | ||
| //check for all-markets |
There was a problem hiding this comment.
@JustinSainton this removal needs to be reviewed
|
I'd like to see this in 4.0. Are all unit tests still passing and is this merge-ready? |
|
|
||
| /* Get country name and symbol */ | ||
| $currency_type = get_option( 'currency_type' ); | ||
| $country = new WPSC_Country( $currency_type ); | ||
| $country = wpsc_get_currency_type_country_object(); |
There was a problem hiding this comment.
I'm not a massive fan of ending function names in data types, e.g. WPSC_Countries::get_countries_array() or this, wpsc_get_currency_type_country_object(). Seems to me it'd be better to just be wpsc_get_currency_type_country?
|
@JeffPyeBrook What's your bandwidth like for this for the 4.1 cycle? |
|
i'm around |
|
Yay! What's your sense on the ETA for the work here? |
|
i'll have to take a look and see what is left to be done, my vague recollection was that it was substantially done. i would guess that the code is in some of my production configurations and has been for a while? |
|
@JeffPyeBrook Let me know if you have an ETA on this - if we can get all the unit tests passing, comments addressed, and PR rebased, it would be great to get this in 4.0. |
JustinSainton
force-pushed
the
master
branch
from
9868144
to
a97791b
Compare
Update countries, regions and currencies to be PHP code based rather than from the database.
This early PR is being made available so that the peer review can start now while work is being completed