Skip to content

Conversation

nicoschtein
Copy link
Contributor

Please review this and test AFTER you have fully reviewed this, since it plays with coin client transactions amounts.

Also check if i'm missing any file that should be modified too.

Closes #875 when merged.

@nicoschtein
Copy link
Contributor Author

Please re test this as most of the files changed since the first tests we made...

@TheSerapher
Copy link
Contributor

Nice. Will check and merge tomorrow!

This will require pool ops to add the new options or pool payouts will run into PHP errors/notices.

@TheSerapher
Copy link
Contributor

Looks to me that your code is still the same. If no veto this will be merged today!

@CaptainAK
Copy link
Contributor

Working for me after manually setting it up. Let's get it merged

@scratchy
Copy link

You forgot the cronjobs ! autotxfee is not working correctly

@scratchy
Copy link

sorry, something went wrong with my setup...

@nicoschtein
Copy link
Contributor Author

I'm pretty sure I modded the cron jobs but let me know if I missed something

N I C O

On Dec 21, 2013, at 9:48 PM, scratchy notifications@github.com wrote:

sorry, something went wrong with my setup...


Reply to this email directly or view it on GitHub.

@nicoschtein
Copy link
Contributor Author

Is this ready to be merged?

@CaptainAK
Copy link
Contributor

Yes, merge it. Been working great on my pools for a while now. A warning might be a good idea though in case someone blindly does a git pull.

@TheSerapher
Copy link
Contributor

Need a quick rebase then I can merge it.

@TheSerapher
Copy link
Contributor

@nicoschtein Please rebase your PR again, push with -f so the PR updates. Then I can merge this into Next! Thanks for your work on this!

@TheSerapher
Copy link
Contributor

Hey @nicoschtein,

I am working on a big feature that will greatly help on merges like this and future other ones: #1246

This will allow us to update the config without breaking a pool for those operators that blindly pulled a change. Crons will be disabled and a big error message will popup for admins warning them to upgrade their system properly.

That's the best we can do for those core changes that can potentially break the system for admins not paying attention. I will merge that sometime next week, maybe we can get back to your update then? Would have no problem to merge it then since we already notify them.

@nicoschtein
Copy link
Contributor Author

It's fine for me wither way, great feature #1246 btw 👍

@TheSerapher
Copy link
Contributor

Okay, let me finish the version checking, then we can merge your rebased branch.

@TheSerapher
Copy link
Contributor

@nicoschtein Version checking is now added! Feel free to modify your branch a bit to use the new config version with your changes so users are notified. At the same time, the rebase could be done.

Thanks a lot!

@TheSerapher
Copy link
Contributor

@nicoschtein awaiting your rebased work and don't forget to increment the version string in the global dist config to 0.0.2 :-)

Merging as soon as that's done.

@nicoschtein
Copy link
Contributor Author

Ok finally I got the time to rebase this here: #1412
Closing this one....

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

Successfully merging this pull request may close these issues.

4 participants