Skip to content
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 code to support more use cases; added wrapper to converted c… #28

Merged
merged 2 commits into from
May 28, 2015

Conversation

phillipgreenii
Copy link
Contributor

…ode to better support special variables (require, module, exports) such as #4

In a previous discussion in gitter we briefly discussed adding support for AMD plugins (#27). I had started work, but never got it working completely. After our discussion, I was no longer able to do a clean merge, so I started re-coding based upon the latest master. This PR is some refactoring that is a mid point between my partial plugin support and the current master. If this is accepted, then I can create another PR for the plugin support.

…ode to better support special variables (require, module, exports) such as jaredhanson#4
@sh1989
Copy link

sh1989 commented May 27, 2015

Great work! Just ran into deamdify not correctly handing files in the following format:
define('name', function() { ... });

...which is demonstrated as fixed in your changes to test/data/definition-name-function.js and test/data/definition-name-function.expect.js 👍

@tbranyen
Copy link
Collaborator

Awesome, I had started on plugin support as well, but if you've got momentum I don't want to stop you. Please start a new branch for the plugins work. This will get merged, but I need to review first as many files have changed.

@tbranyen
Copy link
Collaborator

Looks good 👍

tbranyen added a commit that referenced this pull request May 28, 2015
refactor code to support more use cases; added wrapper to converted c…
@tbranyen tbranyen merged commit 909dc34 into jaredhanson:master May 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants