Torann / laravel-currency

This provides Laravel with currency functions such as currency formatting and conversion using up-to-date exchange rates.
http://lyften.com/projects/laravel-currency
BSD 2-Clause "Simplified" License
393 stars 136 forks source link

Implement 4 sources of exchange rates, contract and manager #114

Open semyonchetvertnyh opened 5 years ago

semyonchetvertnyh commented 5 years ago
  1. https://exchangeratesapi.io
  2. https://openexchangerates.org
  3. https://currencylayer.com
  4. https://fixer.io
use Torann\Currency\Facades\SourceManager;

// Option 1
SourceManager::fetchFrom('fixer')->update();

// Option 2
SourceManager::source('fixer')->update();

// Option 2
$manager = app(\Torann\Currency\SourceManager::class);
$manager->fetchFrom(CustomImplementationOfSourceContract::class);
$manager->update();

// Option 4
SourceManager::extend('customApiService', function () {
    return new CustomApiService;
});

SourceManager::fetchFrom('customApiService')->update();
semyonchetvertnyh commented 5 years ago

@Torann take a look please

Torann commented 5 years ago

This looks great! There's a lot here so I'm going to have to pull it down and play with it some before I approve.

semyonchetvertnyh commented 5 years ago

Ok, will waiting for your answer!

semyonchetvertnyh commented 5 years ago

Hey @Torann!

I just committed some changes to make the code a bit easier to read. Still waiting for you answer :)

Btw to see the difference, it's better to choose "Hide whitespace changes" in "Files changed" tab.

f-liva commented 5 years ago

Is this ready to be merged?

semyonchetvertnyh commented 5 years ago

In my opinion - yes, it is. But @Torann should make code review.

f-liva commented 5 years ago

Let's hope he takes care of it soon then

Torann commented 5 years ago

At a glance there is a lot going on here. I will need to sit down and make sure this doesn't affect current installs of the package (backwards compatible), if it does we will need an upgrade doc and it will be given a new version. Great work!

f-liva commented 5 years ago

Any news here?

Torann commented 5 years ago

There is a lot of changes here. I will need to sit down and test them all before I merge. The quick once over I did of the code though looks great!

klimenttoshkov commented 4 years ago

@semyonchetvertnyh Семьон, have you considered releasing own version? I really would like to use this patched version but it's not possible unless it's either merged or released as another package.

semyonchetvertnyh commented 4 years ago

@klimenttoshkov I don't have plans to create and maintain my own version of the package. If you want to use a patched version, you could fork this branch and use directly in your project.

Just add in your composer.json a path to your GitHub repository:

    "require": {
        "torann/currency": "dev-master",
    },
    "repositories": [
        {
            "url": "https://github.com/klimenttoshkov/laravel-currency.git",
            "type": "git"
        }
    ]
klimenttoshkov commented 4 years ago

Already did that, thank you for the advice! — Kliment Toshkov mail@klimenttoshkov.com

On 15 May 2020, at 18:27, Semyon Chetvertnyh notifications@github.com wrote:

@klimenttoshkov https://github.com/klimenttoshkov I don't have plans to create and maintain my own version of the package. If you want to use a patched version, you could fork this branch and use directly in your project.

Just add in your composer.json a path to your GitHub repository:

"require": {
    "torann/currency": "dev-master",
},
"repositories": [
    {
        "url": "https://github.com/klimenttoshkov/laravel-currency.git",
        "type": "git"
    }
]

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Torann/laravel-currency/pull/114#issuecomment-629307962, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOIQGVJSNH62X4YGNC6TJ6DRRVNMLANCNFSM4GJHUVTQ.

tariqdev commented 4 years ago

@Torann any update on this please

klimenttoshkov commented 3 years ago

@semyonchetvertnyh can you please help with this issue on Laravel 8:

Undefined property: Torann\Currency\SourceManager::$app

Thank you!

semyonchetvertnyh commented 3 years ago

@klimenttoshkov I guess property $app was renamed to $container in abstract Manager class of Laravel 8.

Just change $this->app to $this->container in your SourceManager class. It should help.