Fixes twitch panel #2618
Fixes twitch panel #2618
Conversation
NejcZdovc
force-pushed
the
twitch-fix
branch
3 times, most recently
from
d453493
to
5f8a1c7
Compare
NejcZdovc
force-pushed
the
twitch-fix
branch
2 times, most recently
from
0111e82
to
0c7792e
Compare
| std::transform(media_id.begin(), | ||
| media_id.end(), | ||
| media_id.begin(), | ||
| ::tolower); |
There was a problem hiding this comment.
Can we use base::ToLowerASCII here?
| return; | ||
| } | ||
| ledger::TwitchEventInfo old_event; | ||
| std::map<std::string, ledger::TwitchEventInfo>::const_iterator iter = |
There was a problem hiding this comment.
Nit: Could use auto here to make this declaration more concise.
|
|
||
| if (publisher_id.empty()) { | ||
| const std::string url = publisher_url + "/videos"; |
There was a problem hiding this comment.
It seems like url isn't used anywhere, unless I'm just missing it.
There was a problem hiding this comment.
We're good on this:
https://github.com/brave/brave-core/pull/2618/files#diff-410ec7b37fbed59610d3617cce571df3R646
it's used outside of diff
vendor/bat-native-ledger/src/bat/ledger/internal/media/twitch.cc
Outdated
Show resolved
Hide resolved
Resolves brave/brave-browser#3590 Resolves brave/brave-browser#4680
|
Quickly went through the test case outlined under #2618 (comment) to make sure things are working before uplifting #2633. Looks like things are generally working. QA will eventually go through a more detailed check/verification. Example of the fix working under
|
Resolves brave/brave-browser#3590
Resolves brave/brave-browser#4680
Resolves brave/brave-browser#3417
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) onnpm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.