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

destroys legacy bounty handling code (except for on legacy kill bounty page) #589

Merged
merged 12 commits into from Mar 15, 2018

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Mar 10, 2018

Description

As an artifact of our integration with standard bounties, we have a straggling bunch of bounties on the old Gitcoin smart contracts.

This PR destroys the legacy bounty handling code.

Still TODO

  • What legacy bounties remain?
  • What can we do to close down those bounties and/or migrate them over to standardbounties? How can we make it easy for the parties involved in those bounties to do so?
Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

bounties (legacy)

Refers/Fixes

#406

@owocki
Copy link
Contributor Author

owocki commented Mar 10, 2018

here is a script i wrote to analyze the currently open legacy bounties

from dashboard.models import Bounty
bounties = Bounty.objects.filter(current_bounty=True, web3_type='legacy_gitcoin', network='mainnet')
bounties = bounties.exclude(idx_status__in=['expired','done'])
for bounty in bounties:
    print(bounty.status, bounty.github_url)
    

and it's output

open https://github.com/nbanmp2/testrepo/issues/3
open https://github.com/TimVanMourik/Porcupine/issues/19
started https://github.com/ethereum/web3.py/issues/549
open https://github.com/ProjectWyvern/dao.projectwyvern.com/issues/1
submitted https://github.com/MetaMask/metamask-extension/issues/437
submitted https://github.com/ConsenSys/Tokens/issues/103
submitted https://github.com/gitcoinco/web/issues/152
open https://github.com/trufflesuite/truffle/issues/678
open https://github.com/gitcoinco/chrome_ext/issues/1
open https://github.com/lynndylanhurley/devise_token_auth/issues/976
submitted https://github.com/gitcoinco/skunkworks/issues/38
open https://github.com/ethereum/pyrlp/issues/44
submitted https://github.com/gitcoinco/web/issues/75
started https://github.com/trufflesuite/ganache-cli/issues/453
submitted https://github.com/gitcoinco/skunkworks/issues/34
open https://github.com/ethereum/remix/issues/218
submitted https://github.com/nbanmp2/testrepo/issues/1
started https://github.com/MetaMask/metamask-extension/issues/2350
submitted https://github.com/gitcoinco/web/pull/48
started https://github.com/btcsuite/btcd/issues/1089

two ideas for allowing people to manage these legacy bounties after this code is gone:

  1. spin up legacy.gitcoin.co -- a site which runs a version of the codebase that is pinned to an old commit hash.. and allows interaction with the smart contracts.
  2. for each legacy bounty... allow some limited functionality associated with recovering your funds [ killing the bounty (or submitting / accepting the funds) ]

@mbeacom
Copy link
Contributor

mbeacom commented Mar 12, 2018

@owocki I'm for 2 - I think option 1 brings us back to our original alternative to maintaining the existing bounties under separate views.
If it means we don't need to run separate apps, I think it would be beneficial to take the time to outline a process or introduce a mechanism to retrieve the funds more easily.

@owocki
Copy link
Contributor Author

owocki commented Mar 12, 2018

If it means we don't need to run separate apps, I think it would be beneficial to take the time to outline a process or introduce a mechanism to retrieve the funds more easily.

this makes sense to me..

maybe destroy all the legacy bounty JS and have the standardbounties JS serve up.. but IFF its a legacy bounty dont display the ability to work on any of the options (i.e. remove 'start work' submit work, accept bounty, etc) and add a button to a one-off page that allows the submitter to recover their funds.

@owocki
Copy link
Contributor Author

owocki commented Mar 13, 2018

just went through and killed all the of the bounties in that list that i owed.. now we are down to

open https://github.com/TimVanMourik/Porcupine/issues/19
open https://github.com/ethereum/pyrlp/issues/44
open https://github.com/lynndylanhurley/devise_token_auth/issues/976
started https://github.com/btcsuite/btcd/issues/1089
open https://github.com/ProjectWyvern/dao.projectwyvern.com/issues/1

@owocki
Copy link
Contributor Author

owocki commented Mar 13, 2018

#589 (comment)

i just went through all of the above comments and asked the developers on them to clawback their bounties this week ( copy )

@owocki
Copy link
Contributor Author

owocki commented Mar 13, 2018

slept on this last night to determine how we can have a bias towards action on this:

i think ill give the above parties until the end of the week (thursday) to kill their bounties.. then (only IFF any of them are leftover) i will code up a page that will allow them to interact with legacy bounty contracts and get their money back (either via kill bounty or via claim / accept).. and make that available live as the last remaining vestige of legacy bounties..

@mbeacom @jasonrhaas does that sound okay to you?

@mbeacom
Copy link
Contributor

mbeacom commented Mar 13, 2018

@owocki Sounds good to me! 🍾

@mbeacom
Copy link
Contributor

mbeacom commented Mar 13, 2018

@jasonrhaas
Copy link
Contributor

@owocki I like it, getting closer to being able to remove legacy support 👍

@owocki
Copy link
Contributor Author

owocki commented Mar 14, 2018

@mbeacom i just completed the functionality on the page thats gonna allow a user to recoup their funds if they have a legacy bounty. see 300d6ce -- look okay?

@owocki owocki changed the title [WIP] destroys legacy bounty handling code destroys legacy bounty handling code (except for on legacy kill bounty page) Mar 14, 2018
@codecov
Copy link

codecov bot commented Mar 14, 2018

Codecov Report

Merging #589 into master will increase coverage by 0.2%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #589     +/-   ##
=========================================
+ Coverage   33.97%   34.18%   +0.2%     
=========================================
  Files          92       90      -2     
  Lines        5053     5362    +309     
  Branches      584      652     +68     
=========================================
+ Hits         1717     1833    +116     
- Misses       3269     3454    +185     
- Partials       67       75      +8
Impacted Files Coverage Δ
app/app/urls.py 100% <ø> (ø) ⬆️
app/dashboard/views.py 21.42% <0%> (+2.86%) ⬆️
app/legacy/urls.py 100% <100%> (ø) ⬆️
app/retail/emails.py 24.13% <0%> (-1.18%) ⬇️
...eting/management/commands/bounty_feedback_email.py 0% <0%> (ø)
app/marketing/mails.py 16.74% <0%> (+3.02%) ⬆️
app/dashboard/router.py 45.28% <0%> (+7.99%) ⬆️
app/retail/views.py 48.87% <0%> (+8.19%) ⬆️

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 73d7ffa...8e82928. Read the comment docs.

@owocki
Copy link
Contributor Author

owocki commented Mar 14, 2018

its weird the isort is faiing on app/marketing/webhookviews.py bc its working on my local

@mbeacom
Copy link
Contributor

mbeacom commented Mar 14, 2018

@owocki Can you update your isort ? I'm thinking it might be a version issue. isort -rc --atomic . pops with that additional blank line on my end.

@owocki
Copy link
Contributor Author

owocki commented Mar 14, 2018

just updated i sort to isort-4.3.4-py3-none-any.whl and pushed a fix

@mbeacom
Copy link
Contributor

mbeacom commented Mar 14, 2018

Looks like it's failing on eslint now. I'm not entirely sure what isort changed/fixed in the latest version, but that looks good 👍

@owocki
Copy link
Contributor Author

owocki commented Mar 15, 2018

just pushed up some eslint fixes

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

Successfully merging this pull request may close these issues.

None yet

3 participants