Skip to content

Add the option to display the Account Estimated Earnings on BTC or in USD - #112

Merged
Evanito merged 8 commits into
BitBotFactory:masterfrom
prfiredragon:master
Oct 6, 2016
Merged

Add the option to display the Account Estimated Earnings on BTC or in USD#112
Evanito merged 8 commits into
BitBotFactory:masterfrom
prfiredragon:master

Conversation

@prfiredragon

@prfiredragonprfiredragon commented Sep 29, 2016

Copy link
Copy Markdown
Contributor

I add the option to display the Account Estimated Earnings on BTC or in USD. The default is BTC.

@prfiredragon
Update lendingbot.py
486b956
Add the option to display the Account Estimated Earnings on BTC or in USD
@prfiredragon
Update lendingbot.js
f7d7887
Add the option to display the Account Estimated Earnings on BTC or in USD
@prfiredragon
Update README.md
dd84f44
Add the option to display the Account Estimated Earnings on BTC or in USD
@prfiredragon
Update lendingbot.py
df1fb06
Changed to IP of the WebServer back to 127.0.0.1
@prfiredragon
Update lendingbot.js
83cae69
Change the decimal places of the USD to 8 again and add the @ at the USD price.
Evanito
Evanito requested changes Sep 30, 2016
View reviewed changes

@EvanitoEvanito left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Did not test, only reviewed] Overall, good feature that I would like to see implemented. I just need one thing or two to be addressed before I can approve.

Comment threadlendingbot.py Outdated

#Check if we need a config file at all (If all settings are passed by args, we won't)
if args.apikey and args.apisecret and args.sleeptimeactive and args.sleeptimeinactive and args.mindailyrate and args.maxdailyrate and args.spreadlend and args.gapbottom and args.gaptop and args.sixtydaythreshold:
if args.apikey and args.apisecret and args.sleeptimeactive and args.sleeptimeinactive and args.mindailyrate and args.maxdailyrate and args.spreadlend and args.gapbottom and args.gaptop and args.sixtydaythreshold and args.nobtc:

@EvanitoEvanitoSep 30, 2016•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't "require" an arg that has toggleable functionality.

@prfiredragonprfiredragonSep 30, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done.

Comment threadlendingbot.py Outdated
log.log("Error canceling loan offer: " + str(e))

#msg = bot.cancelLoanOffer(cur,offer['id'])
#log.cancelOrders(cur, msg)

@EvanitoEvanitoSep 30, 2016•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this need to be changed?

Have you experienced errors with this code? It seems bundled in with the USD feature.

@prfiredragonprfiredragonSep 30, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was testing (#107), I already remove it.

@prfiredragon
Update lendingbot.py
10e39e9
Don't "require" an arg that has toggleable functionality.
@prfiredragon
Update lendingbot.py
f4e7fce
Remove the cancelLoanOffer test.
@rnevet

rnevet commented Sep 30, 2016

Copy link
Copy Markdown
Collaborator

I would like to have a more generic approach, where we can offer to switch the currency in the html. Someone might be more interested in ETH or EUR (if available).

@rnevet

rnevet commented Sep 30, 2016

Copy link
Copy Markdown
Collaborator

And I wouldn't name USDT -> USD, not exactly the same and no reason to.

@Evanito

Evanito commented Sep 30, 2016•
edited
Loading

Copy link
Copy Markdown
Member

That is a very good idea, you can change the config option to

# The currency that the HTML Overview will present the earnings summary in.
#Options are BTC, USDT, ETH or anything as long as it has a direct BTC market.
outputCurrency = BTC

Doing any other currencies that don't have a direct Poloniex market (Such as EUR) will require more overhead and we would need to agree on a source api, I recommend Preev.

@rnevet

rnevet commented Sep 30, 2016

Copy link
Copy Markdown
Collaborator

Lets stick to Polo markets then.

@prfiredragon

prfiredragon commented Sep 30, 2016

Copy link
Copy Markdown
ContributorAuthor

Ok. Good idea, I am going began to test with BTC, USDT, ETH, XMR and DOGE.

@rnevet

rnevet commented Sep 30, 2016

Copy link
Copy Markdown
Collaborator

Cool!

@prfiredragon

prfiredragon commented Oct 1, 2016

Copy link
Copy Markdown
ContributorAuthor

That was interesting. Done. The options are BTC, USDT and all in the BTC market.

@Evanito

Evanito commented Oct 1, 2016

Copy link
Copy Markdown
Member

I have reviewed the code and everything seems to be in order. Though with these kinds of changes I will want some user testing before merge. I will do it tomorrow if Raanan doesn't beat me to it.
Great job and cheers!

@EvanitoEvanito self-assigned this Oct 1, 2016
@prfiredragon

prfiredragon commented Oct 1, 2016

Copy link
Copy Markdown
ContributorAuthor

Thank You. You are welcome.

This was referenced Oct 5, 2016
Closed
Merged
@Evanito
Evanito merged commit 4955710 into BitBotFactory:masterOct 6, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@EvanitoEvanitoEvanito approved these changes

Assignees

@EvanitoEvanito

Labels

Enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@prfiredragon@rnevet@Evanito