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

Added the ability to have code based configuration rather than file based #103

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stevechilds
Copy link

In Addition logfiles now no longer have to exist, they are created upon first use.

…ased.

Logfiles now no longer have to exist, they are created upon first use.
@Peardian
Copy link
Collaborator

Thank you for the contribution! I don't have time to fully check it just yet, but it looks like Travis already caught a small problem. Until I am able to test it, would you mind fixing the issue and any others that pop up in Travis?

@stevechilds
Copy link
Author

stevechilds commented Sep 27, 2016

No problem. I did run it through the tests, but not Travis, but it's about time I looked into Travis anyway :)

Regards

Steve

Copy link
Collaborator

@Peardian Peardian left a comment

Choose a reason for hiding this comment

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

I think I did this correctly

}
if (isset($logfunction) && $logfunction != '' && function_exists($logfunction)){
$logCallback = $this->config->getLogCallback();
if (empty($this->config->getLogCallback()) === false){
Copy link
Collaborator

Choose a reason for hiding this comment

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

PHP 5.4 will give a fatal error on this

@@ -2,6 +2,11 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

# 1.4.0 - 2016-08-27
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you don't mind, I would prefer it if changes to the changelog were handled by me at the time of release.

@@ -1 +1,2 @@
The phpAmazonMWS library was designed and written by Thomas Hernandez (peardian at gmail) for the CPI Group.
The v1.4.0 refactoring when adding the AmazonMWSConfig class was done by Steve Childs (stevechilds76 at gmail) for Color Confidence (UK).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do want to give contributors credit, but I haven't yet figured out the best way to go about it, so for now I would prefer it if the credits file was not modified. You are welcome to put an author tag in the new config class, though.

@Peardian
Copy link
Collaborator

Since the project has Travis support added, checking Travis is easy. All you have to do is click the red X (or green mark) next to the commit hash to see the test results.

@stevechilds
Copy link
Author

stevechilds commented Sep 27, 2016

They're all fine, I did wonder about the credits and change log, I just figured if you didn't want them, you'd revert them ;)

As for that bug, you're right, I missed that one it's obviously not covered by the tests, I'll fix that too when I'm back at work.

Regards

Steve

@stevechilds
Copy link
Author

Should hopefully all be fixed now :)

@Peardian
Copy link
Collaborator

Thanks. I will get to checking it in-depth when I able to.

@stevechilds
Copy link
Author

stevechilds commented Sep 29, 2016

My pleasure @Peardian, thanks for creating the library in the first place, it's saved me a lot of time (even allowing for the half day to refactor the code ;) ).

When I've got some time I'll add some Symfony usage examples too, even if its just a 'how-to' for inclusion in the read-me, it may be useful to someone :)

@ThomasRedstone
Copy link

This is great, solves a real problem for me, is there any estimate on when it will be released @Peardian?

@Peardian
Copy link
Collaborator

Peardian commented Oct 6, 2016

@ThomasRedstone If I can't get to it next week, it may have to be sometime in November.

@stevechilds
Copy link
Author

@ThomasRedstone - Feel free to use my fork (I think its public) in the mean time if you can't wait. I've been using it myself and I haven't found any issues yet... (Famous last words, I know!)

@Peardian
Copy link
Collaborator

I've finally had a chance to sit down and look through it in detail. It's good for the most part, but there are some tweaks I want to make to the design before it gets approved. When I have the time to work on it, I'll make pull request based off of your branch. I'll leave this open until then. Thanks again for the contribution!

@Peardian
Copy link
Collaborator

Drats, I should have known that the other pull request would conflict with this one. I'll fix them when I make my tweaks.

@Peardian Peardian changed the base branch from stable to master November 3, 2016 15:32
@mpixelz
Copy link

mpixelz commented Nov 21, 2016

hi.. any chance this will be done anytime soon?

@Peardian
Copy link
Collaborator

I have been busy with other things in preparation for the holiday season. Once those are done, I may have time to work on this, but I can't make any guarantees until Christmas has passed.

@stevechilds
Copy link
Author

Please Note, I've changed my username, so my fork is now at https://github.com/stevechilds/phpAmazonMWS

@mpixelz
Copy link

mpixelz commented Jan 16, 2017

Hi,
just wanted to know when can we get this merged? :)
thanks.

@Peardian
Copy link
Collaborator

Unfortunately, I have not had a chance to work on this yet. I am still busy with wrapping up some holiday-time projects, but I may have some time to work on it at the end of the month or next month.

@ahmettok
Copy link

ahmettok commented Mar 3, 2017

I think this is a great contribution to the project.
Any idea when you will merge?
I will use /stevechilds/phpAmazonMWS in the meantime.

@Peardian
Copy link
Collaborator

Peardian commented Mar 3, 2017

I don't have a solid answer, but I am hoping to work on it sometime this month. Only just last week was I able to finish up the projects I mentioned in my previous comment, so I hope to find time to work on this relatively soon.

@ahmettok
Copy link

ahmettok commented Mar 7, 2017

Thank you, let me know if I can help with anything!

@mpixelz
Copy link

mpixelz commented Mar 26, 2017

Hi @Peardian
hope you are doing good.. just wanted to ask whats an eta on this merge?
really looking forward on getting this merged :)
much appreciated on all the work you have already done :) saved me alot of time and effort :)
thanks.

@Peardian
Copy link
Collaborator

Unfortunately, I still haven't had a chance to work on this. I have been much busier than I anticipated with new projects. I want to work on this soon, though that is not entirely in my control. Maybe this summer I will have time.

@Peardian
Copy link
Collaborator

Peardian commented Apr 21, 2017

I finally have a bit of time to look into this. I don't think I'll be able to finish it today, but at the very least I can probably get a WIP pull request up.

Edit: I didn't get nearly as much time as I anticipated, and don't have anything presentable or functional yet. But rest assured, I have started tinkering with this.

@mpixelz
Copy link

mpixelz commented Apr 25, 2017

Thanks @Peardian
your efforts are much appreciated! :)
looking forward.
cheers

@Peardian
Copy link
Collaborator

Peardian commented May 23, 2017

I am still continuing to look into this with the free time I can manage, though I don't think it will be part of the v1.4. I want to get support for new operations and fields in a stable release as soon as possible, and then this config overhaul can be part of v1.5.

@JonLaliberte
Copy link

Thanks again for all the work. Definitely looking forward to the config change being merged. Currently having to use the forked version for a few projects and would rather get back on the mainline. Again though, thanks for all the work, it makes working with MWS much more pleasant.

@dustinmoore
Copy link

Is there any ETA on this ever getting merged? I think using the forked version would potentially not be up to date with the most recent changes to Amazons API as compared to the master branch?

@Peardian
Copy link
Collaborator

Unfortunately, I have been quite busy and have not had a chance to work on this more. I can't make any promises on when it will be ready, but I plan for this to be the focus of the next release. I realize that it has been over a year now, and I apologize to everyone who has been patiently waiting on this feature.

@cgsmith
Copy link

cgsmith commented Feb 5, 2019

ETA on getting this merged?

@Peardian
Copy link
Collaborator

Peardian commented Feb 5, 2019

Sadly, I haven't had a chance to work on this in over two years. Too many important projects taking up my time. I still intend to get to it eventually.

@mrvnklm
Copy link

mrvnklm commented Feb 5, 2019

For all of you looking for some solution: I use rapidwebltd/amazon-mws-config-generator to use the bundle with symfony.
You have the ability to generate config files on the fly so handling multiple users is not a problem anymore, so at least it's a temporary problem solving solution.

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

Successfully merging this pull request may close these issues.

9 participants