-
Notifications
You must be signed in to change notification settings - Fork 136
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
Implement 4 sources of exchange rates, contract and manager #114
base: master
Are you sure you want to change the base?
Conversation
4 sources: currencylayer.com, exchangeratesapi.io, fixer.io, openexchangerates.org; Add Exceptions
@Torann take a look please |
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. |
Ok, will waiting for your answer! |
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. |
Is this ready to be merged? |
In my opinion - yes, it is. But @Torann should make code review. |
Let's hope he takes care of it soon then |
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! |
Any news here? |
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! |
@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. |
@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 "require": {
"torann/currency": "dev-master",
},
"repositories": [
{
"url": "https://github.com/klimenttoshkov/laravel-currency.git",
"type": "git"
}
] |
Already did that, thank you for the advice!
—
Kliment Toshkov
[email protected]
… On 15 May 2020, at 18:27, Semyon Chetvertnyh ***@***.***> 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 <#114 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOIQGVJSNH62X4YGNC6TJ6DRRVNMLANCNFSM4GJHUVTQ>.
|
@Torann any update on this please |
@semyonchetvertnyh can you please help with this issue on Laravel 8:
Thank you! |
@klimenttoshkov I guess property Just change |
Adding clear exceptions
Adding env variables to config file
Switching from option to --source argument in Update command
Adding Manager and Facade
Adding possibility to update exchange rates from everywhere