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

Enabling brave rewards for Windows #465

Merged
merged 3 commits into from
Sep 26, 2018
Merged

Conversation

gdregalo
Copy link
Contributor

@gdregalo gdregalo commented Sep 18, 2018

This is my first PR to brave-core. I don't know how to handle all the items below. Please someone explain. Please review the code changes. It enables and compiles brave_rewards service into the browser. It pull the latest bat-native-anonize, bat-native-tweetnacl and bat-native-ledger into the project. The bat-native-anonize is still under security review.
It has not been tested thoroughly though. Thanks,

Resolves brave/brave-browser#1165

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Needed or QA/No-QA-Needed) to include the closed issue in milestone

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@NejcZdovc
Copy link
Contributor

@gdregalo created issue in brave browser for this PR

@NejcZdovc
Copy link
Contributor

@gdregalo because of native ledger commit I needed to create separate PR that just changes result values #473

@@ -125,7 +127,13 @@ bool PublisherInfoBackend::EnsureInitialized() {

leveldb_env::Options options;
options.create_if_missing = true;
#if defined(OS_WIN) && (defined(UNICODE) || defined(_UNICODE))
Copy link
Collaborator

Choose a reason for hiding this comment

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

#if defined(OS_WIN)
  std::string path = base::UTF16ToUTF8(path_.value());
#else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amended.

@NejcZdovc NejcZdovc force-pushed the win_brave_rewards_enabled branch from d08c021 to 3999e76 Compare September 20, 2018 07:33
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Sep 20, 2018

@gdregalo just a heads up, I just rebased this PR to the latest master. Will check out now if everything works on windows

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

Run it on windows and it's working. Wallet was created and I could claim grant.

Run npm run test -- brave_unit_tests and it's passing.

Run npm run test -- brave_browser_tests and it's failing on one test

1 test crashed:
    BraveRewardsBrowserTest.ToggleAutoContribute (../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:228)

Blocked on security review

@NejcZdovc
Copy link
Contributor

@jasonrsadler please check this failing test

@NejcZdovc
Copy link
Contributor

pulled 3999e766f1d451f48e7285da0b32faae815fff39 into #487

@NejcZdovc NejcZdovc force-pushed the win_brave_rewards_enabled branch from 3999e76 to 5e4267b Compare September 21, 2018 17:55
@bbondy bbondy merged commit 985746c into master Sep 26, 2018
bbondy added a commit that referenced this pull request Sep 26, 2018
@bbondy
Copy link
Member

bbondy commented Sep 26, 2018

master 985746c
0.55.x 148c270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Rewards on Windows
4 participants