-
Notifications
You must be signed in to change notification settings - Fork 69
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
Refactor plugins as of #112 and fix plugin diff with no fixtures #118
Conversation
Last updated: 5/16/2017, 9:05:42 PM Diff plugin outputs testModified plugin
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely like the cleaner code. Can we somehow prevent the trailing space in self-closing tags from being deleted (<tag/>
instead of <tag />
)? Maybe patch the dependency?
plugins/ecue.js
Outdated
@@ -3,30 +3,45 @@ const path = require('path'); | |||
const util = require('util'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this unused import
plugins/qlcplus.js
Outdated
@@ -82,23 +97,29 @@ module.exports.export = function exportQLCplus(library, options) { | |||
|
|||
function exportHandleAvailableChannels(fixture, defaults) { | |||
let str = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this unused variable
https://github.com/oozcitak/xmlbuilder-js/blob/master/src/XMLStringWriter.coffee#L130 There currently is no way to add the additional space without making changes to the dependency. But we could create an issue and ask whether it's possible/wished to add an option to the dependency that adds this extra space and then make a pull request implementing that functionality. However, it might be considerable whether this space is really that important. |
…ure-library into plugins-xmlbuilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we merge this now and add the space option later if possible.
No description provided.