search

egh / ledger-autosync

Synchronize your ledger-cli files with your bank.
GNU General Public License v3.0
273 stars 42 forks source link

Always use ticker symbol #12

Closed egh closed 7 years ago

egh commented 8 years ago
egh commented 8 years ago

@jblachly Does this change make sense to you?

jblachly commented 8 years ago

@egh, interestingly that is the original solution I created (in my comments I referred to "conversion at point-of-use"). This does have the advantage of not relying on "magic," but does mean more potential work in the future. For example, the commit in this pull request does not change the ticker symbol in the position statement printouts:

https://github.com/egh/ledger-autosync/blob/5a856f71352a44db38b3108ea97895d899e67d18/ledgerautosync/converter.py#L379

So, you'll need to do the same cusip->ticker remapping inside that function ... but also anywhere else that in the future uses the ticker symbol. If you'e okay with that, then roll with it because I admit the remapping code that rewrites the OFX object is ugly!

egh commented 8 years ago

@jblachly Great, thanks! I thought that might have been what you'd originally coded. I think it's a little simpler and it shouldn't be slower. Adapting it to price statements was pretty easy as well.

jblachly commented 7 years ago

@egh looks great, love the function name! LGTM, Ship it! (edit because I wrote "Looks Good Me To" :| )

egh commented 7 years ago

Great, thanks for taking the time to look at over and find what I missed! I really appreciate all the help.