-
Notifications
You must be signed in to change notification settings - Fork 75
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
[WIP] Remove to_xml #430
[WIP] Remove to_xml #430
Conversation
@miq-bot add_label technical debt |
Pull Request Test Coverage Report for Build 4367
💛 - Coveralls |
@mkanoor Any concern with trying to get rid of all the XML stuff on master? It likely needs to happen over the course of a few PRs, but feels like a good technical debt cleanup. Thoughts? |
@gmcculloug I think the UI uses this to display the output of simulation, we would have to coordinate with them to start using JSON instead of XML when displaying the simulation output. |
@gmcculloug we don't really need the YAML or JSON version, I can just build the tree directly from the objects. Faster to build directly and avoid parsing of formats... |
f7ec387
to
98b04c8
Compare
there's also |
That's the one I'm dependent on in the UI, but it's getting close... |
gotcha, thanks |
98b04c8
to
c85e83b
Compare
it's fine, this's got that failing spec that i'm looking into, there's no rush |
Orienting in the code is way too complicated for me, so maybe @mkanoor's idea about you implementing a |
How about xml_import? Do we not remove it this time? |
c85e83b
to
9a57516
Compare
Checked commit d-m-u@9a57516 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
I don't think we can make this change without an implementation of |
This is follow-up to #429.
This was what I could remove without specs failing, and it's kinda late, so I'll get back to it tomorrow.
there are a few more:
but with those removed as well,
manageiq-automation_engine/spec/miq_ae_state_machine_spec.rb
Line 37 in 1a3ba3a
ae_result
oferror
rather thanok
.Maybe also
app/models/miq_ae_datastore/xml_import.rb
can go, but I have to look closer at it tomorrow cause it causesuninitialized constant
constant problems