Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CSL-2343] Remove wallet/node directory #2643

Closed
wants to merge 1 commit into from

Conversation

gromakovsky
Copy link
Contributor

It's not used anymore, so I suppose it should be removed as part of CSL-2343.

It's not used anymore, so I suppose it should be removed as part of CSL-2343.
@Hithroc
Copy link
Contributor

Hithroc commented Mar 8, 2018

Should scripts/launch/demo-with-new-wallet-api.sh also be removed? Asking here because this seems like relevant pull request.

@gromakovsky
Copy link
Contributor Author

Hm, yes, I think so.

@gromakovsky
Copy link
Contributor Author

Actually no, I don't think so, because there is --new-wallet flag in it. So probably it should be left for now.

Copy link
Contributor

@akegalj akegalj left a comment

Choose a reason for hiding this comment

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

I believe this is good. Before merging I would wait for some time so that we are sure nothing is broken so that we can easily revert. For now only with https://iohk.myjetbrains.com/youtrack/issue/CSL-2343 - but we don't want to make more of similar PRs (that would be harder to track to revert).

For example - the renaming is also not working as expected https://iohk.myjetbrains.com/youtrack/issue/CSL-2364

In short: looks good but lets not pull the trigger just yet

Copy link
Contributor

@adinapoli-iohk adinapoli-iohk left a comment

Choose a reason for hiding this comment

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

I think this is fine. My preference would be to pull the trigger instead. It's a single commit anyway, that's trivial to revert. Will leave @gromakovsky to decide.

@adinapoli-iohk
Copy link
Contributor

@KtorZ @parsonsmatt I think this PR is worthwhile reviving, as we removed the executable flag from wallet.cabal but we are still keeping the content inside wallet/node/* alive for no good reason.

Would you resurrect this one and give it another spin?

@akegalj akegalj mentioned this pull request Jun 18, 2018
11 tasks
@akegalj
Copy link
Contributor

akegalj commented Jun 18, 2018

Closed in favour of #3104

@akegalj akegalj closed this Jun 18, 2018
@akegalj akegalj deleted the gromak/csl2343-remove-wallet/node branch June 18, 2018 14:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants