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

Manual allocation #41

Merged
merged 22 commits into from
Jan 22, 2018
Merged

Manual allocation #41

merged 22 commits into from
Jan 22, 2018

Conversation

gosuto-inzasheru
Copy link
Contributor

Quick solution to #40. Note that this does not allow you to set percentages directly, merely give each coin 'points' which are calculated to percentages based on the total points given to all coins that are held.

@benmarten
Copy link
Owner

Hi, thanks for this commit. Makes a lot of sense....
I'm still a bit hesitant to merge this. I feel we should at least support percentages, otherwise it could be a bit confusing...
We should also add at least an entry in the readme and in the settings.example.json.

I think we can potentially address this with the Strategy class that I mentioned earlier... ;)

Copy link
Owner

@benmarten benmarten left a comment

Choose a reason for hiding this comment

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

see comment

@benmarten benmarten force-pushed the develop branch 2 times, most recently from ef42ccf to d5ab3e7 Compare January 22, 2018 08:07
@gosuto-inzasheru
Copy link
Contributor Author

In a way percentages is supported. If you fill in 66.66 and 33.33 it will do the trick (actually rebalance to 66.67 and 33.33). It's just that with this factor calculating it is just as easy to fill in 2 and 1 or 6 and 3. No errors can be made either, allocation will never go above 100% for example. Whatever is filled in manually, the allocation sum will always be 100%. I think this is therefore a more fool proof way of handeling manual data...

I will check out the conflict in README.md.

@gosuto-inzasheru
Copy link
Contributor Author

gosuto-inzasheru commented Jan 22, 2018

Should be good now, following ca7929f.

@benmarten
Copy link
Owner

Looks good now. Thx

@benmarten benmarten merged commit f47a63f into benmarten:develop Jan 22, 2018
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