{"action":"created","issue":{"url":"https://api.github.com/repos/CityOfZion/neo-python/issues/733","repository_url":"https://api.github.com/repos/CityOfZion/neo-python","labels_url":"https://api.github.com/repos/CityOfZion/neo-python/issues/733/labels{/name}","comments_url":"https://api.github.com/repos/CityOfZion/neo-python/issues/733/comments","events_url":"https://api.github.com/repos/CityOfZion/neo-python/issues/733/events","html_url":"https://github.com/CityOfZion/neo-python/pull/733","id":386551452,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM1MjUzNTMz","number":733,"title":"[refactor-prompt] Add missing tests for wallet create, verbose and create_addr","user":{"login":"LysanderGG","id":677006,"node_id":"MDQ6VXNlcjY3NzAwNg==","avatar_url":"https://avatars0.githubusercontent.com/u/677006?v=4","gravatar_id":"","url":"https://api.github.com/users/LysanderGG","html_url":"https://github.com/LysanderGG","followers_url":"https://api.github.com/users/LysanderGG/followers","following_url":"https://api.github.com/users/LysanderGG/following{/other_user}","gists_url":"https://api.github.com/users/LysanderGG/gists{/gist_id}","starred_url":"https://api.github.com/users/LysanderGG/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LysanderGG/subscriptions","organizations_url":"https://api.github.com/users/LysanderGG/orgs","repos_url":"https://api.github.com/users/LysanderGG/repos","events_url":"https://api.github.com/users/LysanderGG/events{/privacy}","received_events_url":"https://api.github.com/users/LysanderGG/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2018-12-02T11:37:21Z","updated_at":"2018-12-05T10:16:34Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/CityOfZion/neo-python/pulls/733","html_url":"https://github.com/CityOfZion/neo-python/pull/733","diff_url":"https://github.com/CityOfZion/neo-python/pull/733.diff","patch_url":"https://github.com/CityOfZion/neo-python/pull/733.patch"},"body":"**What current issue(s) does this address, or what feature is it adding?**\r\nAdd missing tests after PR #720 (also see https://github.com/CityOfZion/neo-python/pull/726#issuecomment-442285227)\r\n\r\nThe tests are for the following commands `wallet create`, `wallet verbose`, `wallet create_addr`.\r\n\r\n**How did you solve this problem?**\r\nAdded some tests.\r\n\r\n**How did you make sure your solution works?**\r\nRun `make test`.\r\n\r\n**Are there any special changes in the code that we should be aware of?**\r\nNo.\r\n\r\n**Please check the following, if applicable:**\r\n\r\n- [x] Did you add any tests?\r\n- [x] Did you run `make lint`?\r\n- [x] Did you run `make test`?\r\n- [x] Are you making a PR to a feature branch or development rather than master?\r\n- [ ] Did you add an entry to `CHANGELOG.rst`? (if not, please do)\r\n\r\n\r\nNote:\r\n\r\n- I couldn't find any other test related to wallet migration so I was unable to test the command `wallet migrate`."},"comment":{"url":"https://api.github.com/repos/CityOfZion/neo-python/issues/comments/444433123","html_url":"https://github.com/CityOfZion/neo-python/pull/733#issuecomment-444433123","issue_url":"https://api.github.com/repos/CityOfZion/neo-python/issues/733","id":444433123,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NDQzMzEyMw==","user":{"login":"ixje","id":6625537,"node_id":"MDQ6VXNlcjY2MjU1Mzc=","avatar_url":"https://avatars0.githubusercontent.com/u/6625537?v=4","gravatar_id":"","url":"https://api.github.com/users/ixje","html_url":"https://github.com/ixje","followers_url":"https://api.github.com/users/ixje/followers","following_url":"https://api.github.com/users/ixje/following{/other_user}","gists_url":"https://api.github.com/users/ixje/gists{/gist_id}","starred_url":"https://api.github.com/users/ixje/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ixje/subscriptions","organizations_url":"https://api.github.com/users/ixje/orgs","repos_url":"https://api.github.com/users/ixje/repos","events_url":"https://api.github.com/users/ixje/events{/privacy}","received_events_url":"https://api.github.com/users/ixje/received_events","type":"User","site_admin":false},"created_at":"2018-12-05T10:16:34Z","updated_at":"2018-12-05T10:16:34Z","author_association":"MEMBER","body":"> I think that the best to do is keep the functionnalities not related to the command line outside of Commands so that these functions can be used elsewhere.\r\n\r\nI actually had that in mind (separate presentation from functionality) before writing my previous comment, but thought it might lead to more confusion at this point. \r\n\r\n> I'm a bit lost with this big discussion; can you\r\nNo worries, same here. This is a good showcase why we should keep PR's small, because discussions can still get big :)\r\n\r\nI think for this one let's get it in and we'll think about moving out the body of `execute()` functions before we do the big merge. That way we can have it in a clean PR which is easier to discuss. @jseagrave21 any objections at this point or things that need solving before merging?\r\n\r\nPS: I added the last paragraph as a TODO item to the original \"CLI refactoring post\""}}
{"actor":{"display_login":"ixje"}}