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.

Sign up for GitHub

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

Jump to bottom

WOC: Update Gitcoin for breaking MetaMask changes #1992

Closed
wants to merge 1 commit into from
Closed

WOC: Update Gitcoin for breaking MetaMask changes #1992

wants to merge 1 commit into from

Conversation

zevaverbach
Copy link
Contributor

@zevaverbach zevaverbach commented Aug 11, 2018 •
edited
Loading

Description

I will replace all direct reliance on injected web3 with a check to request permission and initialize web3 manually.

NOTE: This will not work with the current release of Metamask, but rather with this branch, which hopefully will be the current release by November 2018.

Checklist
  • New Bounty works without injected web3
  • Submit Bounty works without injected web3
  • Accept Bounty works without injected web3
  • New Tip works without injected web3
  • Receive Tip works without injected web3
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)
  • UI
Testing
Refers/Fixes

Fixes #1964 (once completed)

@codecov
Copy link

codecov bot commented Aug 11, 2018 •
edited
Loading

Codecov Report

Merging #1992 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1992   +/-   ##
=======================================
  Coverage   25.41%   25.41%           
=======================================
  Files         132      132           
  Lines       10842    10842           
  Branches     1449     1449           
=======================================
  Hits         2755     2755           
  Misses       8009     8009           
  Partials       78       78

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6648b63...38653ed. Read the comment docs.

@zevaverbach zevaverbach mentioned this pull request Aug 11, 2018
Closed
mbeacom
mbeacom suggested changes Aug 12, 2018
View reviewed changes
app/assets/v2/css/base.css
@@ -652,7 +652,7 @@ input[type=text].loading {
color: #4aeba8;
}

.submit_bounty select[name=deonomination] {
.submit_bounty select[name=denomination] {
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to update your master branch and this PR branch from our upstream to remove these previously introduced line changes.

@frankchen07
Copy link
Contributor

hey @zevaverbach, wanted to check in and see if you were able to give @mbeacom's comments some thought! I've snoozed Gitcoin bot for you on the main issue page.

@zevaverbach
Copy link
Contributor Author

zevaverbach commented Aug 29, 2018 via email

@vs77bb
Copy link
Contributor

vs77bb commented Aug 29, 2018

Thanks for the update @zevaverbach!

@owocki
Copy link
Contributor

owocki commented Sep 12, 2018

hey @zevaverbach are you still interested in this?

@mbeacom mbeacom changed the title [WIP] Update Gitcoin for breaking MetaMask changes WOC: Update Gitcoin for breaking MetaMask changes Sep 13, 2018
@owocki
Copy link
Contributor

owocki commented Oct 6, 2018

@zevaverbach ?

@zevaverbach
Copy link
Contributor Author

zevaverbach commented Oct 11, 2018 via email

@owocki
Copy link
Contributor

owocki commented Oct 11, 2018

@zevaverbach how late in October? dropdead date for this is November 2nd per the metamask post

@zevaverbach
Copy link
Contributor Author

zevaverbach commented Oct 12, 2018 via email

@owocki
Copy link
Contributor

owocki commented Oct 12, 2018 via email

@owocki
Copy link
Contributor

owocki commented Oct 22, 2018

FYI -- theres a new post from metamask about how to update dapps to the new metamask => https://medium.com/metamask/eip-1102-preparing-your-dapp-5027b2c9ed76

@mbeacom
Copy link
Contributor

mbeacom commented Nov 29, 2018

Addressed in #2548

@mbeacom mbeacom closed this Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbeacom mbeacom mbeacom requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zevaverbach @frankchen07 @vs77bb @owocki @mbeacom