Skip to content
This repository was archived by the owner on Nov 15, 2021. It is now read-only.

Conversation

@jseagrave21
Copy link
Contributor

@jseagrave21 jseagrave21 commented Dec 4, 2018

What current issue(s) does this address, or what feature is it adding?

How did you solve this problem?
Trial and Error

How did you make sure your solution works?
Unittest

Are there any special changes in the code that we should be aware of?
No

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

Merge CoZ refactor-prompt into jseagrave21 refactor-prompt
- add CommandShow and CommandSearch to prompt
revert changes
- add CommandShow and CommandSearch to prompt
- add Search.py and Show.py
- add test_search_commands.py and test_show_commands.py
- add ShowAllAssets
- add a test for ShowAllAssets
- add ShowAllAssets
@jseagrave21 jseagrave21 changed the title Adds support and tests for the show method group and the search method group [refactor prompt] Adds support and tests for the show method group and the search method group Dec 4, 2018
@ixje
Copy link
Member

ixje commented Dec 5, 2018

Please split this PR into multiple. It's too big to review without losing track if changes are requested or requiring a huge effort to do so. Split it at least into the show and search group commands. Preferably split show into 2 PR's as that would already be 5 commands per PR (you might have to PR the second one after the first is in). Thanks!

@jseagrave21
Copy link
Contributor Author

jseagrave21 commented Dec 5, 2018

Please split this PR into multiple. It's too big to review without losing track if changes are requested or requiring a huge effort to do so. Split it at least into the show and search group commands. Preferably split show into 2 PR's as that would already be 5 commands per PR (you might have to PR the second one after the first is in). Thanks!

Okay, please confirm I should refocus this PR then have two follow on like so?

  • show part 1
    • show part 2
  • search

☝️ In series, not parallel (would be impossible for show part 1/2)

@ixje
Copy link
Member

ixje commented Dec 5, 2018

You can PR search + show part 1 in parallel. They're separate groups so can be separate PR's. Then show part 2 comes later.

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.

2 participants