Differential D3870

[Converter Runner] Also convert to the user's local currency
ClosedPublic
Actions

Authored by broulik on Dec 30 2016, 1:35 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Commits
R114:8fea513310f2: [Converter Runner] Also convert to the user's local currency
Summary

The "most common units" for currency listed by KUnitConversion are EUR, USD, GBP, CAD, JPY but the local currency is probably what the user cares most about.

BUG: 374275
FIXED-IN: 5.9.0

Test Plan

LC_ALL=NO_no krunner
typed "5 eur"
got an additional 45 NOK result

Diff Detail

Repository
R114 Plasma Addons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik updated this revision to Diff 9487.Dec 30 2016, 1:35 PM
broulik retitled this revision from to [Converter Runner] Also convert to the user's local currency.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added a reviewer: Plasma.
broulik set the repository for this revision to R114 Plasma Addons.
Restricted Application added a project: Plasma. · View Herald TranscriptDec 30 2016, 1:35 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson accepted this revision.Dec 30 2016, 1:57 PM
davidedmundson added a reviewer: davidedmundson.
davidedmundson added a subscriber: davidedmundson.

not units.prepend(localCurrency) so it's first?

(Slightly offtopic:
when Martin G, eike or I make a diff, there's always an option to expand the context in phab to view lines above, when Kai or Subdiff make one, it says "Context not available" above the diff and I have to open files manually to work out what the code is doing. Does anyone know what's being done differently ?)

This revision is now accepted and ready to land.Dec 30 2016, 1:57 PM
romangg added a subscriber: romangg.Dec 30 2016, 3:49 PM
In D3870#72396, @davidedmundson wrote:

(Slightly offtopic:
when Martin G, eike or I make a diff, there's always an option to expand the context in phab to view lines above, when Kai or Subdiff make one, it says "Context not available" above the diff and I have to open files manually to work out what the code is doing. Does anyone know what's being done differently ?)

Maybe because I don't use Arcanist? I always did a git diff master > diff_file

elvisangelaccio added a subscriber: elvisangelaccio.Dec 30 2016, 3:57 PM
In D3870#72454, @subdiff wrote:
In D3870#72396, @davidedmundson wrote:

(Slightly offtopic:
when Martin G, eike or I make a diff, there's always an option to expand the context in phab to view lines above, when Kai or Subdiff make one, it says "Context not available" above the diff and I have to open files manually to work out what the code is doing. Does anyone know what's being done differently ?)

Maybe because I don't use Arcanist? I always did a git diff master > diff_file

Yes. You can use (for example) git diff master -U99 > file as workaround. It will generate a diff with 99 lines of context.

broulik added a comment.Dec 30 2016, 6:07 PM

not units.prepend(localCurrency) so it's first?

That's what I did originally but since we sort by magnitude/value at the end, the order would be lost eventually.

(Reviewboard manages to pick context from the repository, Phab only does so *after* having committed… I'll try the "-U" workaround, though)

Closed by commit R114:8fea513310f2: [Converter Runner] Also convert to the user's local currency (authored by broulik). · Explain WhyDec 30 2016, 9:09 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 9523

View Options

runners/converter/converterrunner.cpp

Loading...