[EmbeddedAnsible] TODO Cleanup #19276
Conversation
...models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb
Outdated
Show resolved
Hide resolved
|
The rest look good to me. |
|
@Fryguy Want me to remove the commits around the contested ones then and we can take a look at those later and merge this as is? |
NickLaMuro
force-pushed
the
embedded_ansible_todo_cleanup
branch
2 times, most recently
from
f6f654c to
575def3
Compare
NickLaMuro
force-pushed
the
embedded_ansible_todo_cleanup
branch
from
575def3 to
bd27e0f
Compare
|
Missed the fact that this was unmergable, so that should be fixed now |
|
Error on CI seems unrelated. |
|
Restarted just the one build job - hoping it passes now. if this (little) kick does not fix it, I'll close/close to (full punt) kick a full build (that will rebase and test against the latest - so it tends to resolve sporadic problems) |
|
@Fryguy bump |
|
This pull request has been automatically closed because it has not been updated for at least 3 months. Feel free to reopen this pull request if these changes are still valid. Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation. |
|
Really? Can we just merge this cleanup? |
|
I guess @kbrock really wants me to merge this... I will look at rebasing this then... |
Uncomments a few tests that just work, and removes the ones that are no longer relevant or tested elsewhere.
This probably is a case we should support, but until it comes up, going to remove this TODO as it doesn't seem like something that is needed or heavily requested.
| @@ -156,18 +156,6 @@ | |||
| described_class.create_in_provider(manager.id, params) | |||
| end.to raise_error(RuntimeError, error_msg) | |||
| end | |||
|
|
|||
| # TODO: Determine if we want to have a uniqueness validation to | |||
There was a problem hiding this comment.
FYI: This file deleted in #19383 so this will not be included as part of the next rebase.
NickLaMuro
force-pushed
the
embedded_ansible_todo_cleanup
branch
from
bd27e0f to
881d6c0
Compare
|
Checked commits NickLaMuro/manageiq@5dabc93~...881d6c0 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
Removes a bunch of TODO comments from the
app/andspec/directories that I felt were not relevant.Of the remaining ones:
The second one is something that needs further thought and discussion on the best way to move forward (service class possibly), so don't want to tackle that in this PR.
Edit: Per the PR review, the ones we did remove from the
app/dir should have some more discussion, so removed those for the time being.