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

Automated migration tools for QUnit 2.0 syntax #868

Closed
platinumazure opened this Issue Oct 2, 2015 · 13 comments

Comments

5 participants
@platinumazure
Contributor

platinumazure commented Oct 2, 2015

Hi folks,

I've been working (occasionally) on a QUnit migrator tool, platinumazure/qunit2-migrator. Today I learned of another tool being developed, apsdehal/qunit-migrate.

Before I get too much further, I'd like to confirm with the QUnit team: Are there any plans to create or support a migration tool? If so, what do I need to do to either win support for what I have done or to gracefully fold in my tool's capabilities to whatever the "official" migration tool may be? And if there are no plans to support a tool, what is the best way for me to request extra eyes on it and get feedback without spamming people?

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Oct 3, 2015

Member

As a quick note: We should add links on both the plugins and the upgrade guide pages to available migration tools.

I'll get back to the actual question later.

Member

jzaefferer commented Oct 3, 2015

As a quick note: We should add links on both the plugins and the upgrade guide pages to available migration tools.

I'll get back to the actual question later.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Oct 6, 2015

Member

It looks like your tool is based on AST transforms, while @apsdehal's tool is using regex. He mentioned (apsdehal/qunit-migrate#4 (comment)) that he'd like to switch to using JS AST, so that seems like a good opportunity to collaborate. It would be great to have one really solid tool instead of two that mostly solve the same problem.

@apsdehal since you mentioned moving to AST, would you consider contributing to @platinumazure's tool?

Member

jzaefferer commented Oct 6, 2015

It looks like your tool is based on AST transforms, while @apsdehal's tool is using regex. He mentioned (apsdehal/qunit-migrate#4 (comment)) that he'd like to switch to using JS AST, so that seems like a good opportunity to collaborate. It would be great to have one really solid tool instead of two that mostly solve the same problem.

@apsdehal since you mentioned moving to AST, would you consider contributing to @platinumazure's tool?

@apsdehal

This comment has been minimized.

Show comment
Hide comment
@apsdehal

apsdehal Oct 7, 2015

@jzaefferer I can contribute to @platinumazure's tool, but the tool I developed came into existence mostly because of the custom needs of jquery-projects and the custom syntax. Thus I further modified it to a more general support. I can't immediately halt the development on it until @platinumazure's tool take care of different syntax, good wikis and a few other stuff. We can work together in that direction.

apsdehal commented Oct 7, 2015

@jzaefferer I can contribute to @platinumazure's tool, but the tool I developed came into existence mostly because of the custom needs of jquery-projects and the custom syntax. Thus I further modified it to a more general support. I can't immediately halt the development on it until @platinumazure's tool take care of different syntax, good wikis and a few other stuff. We can work together in that direction.

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Oct 7, 2015

Contributor

@apsdehal The syntax tree transformation I selected, recast, was chosen specifically to try to preserve existing syntax and spacing and things, although it does have a few bugs when new statements are generated. Could you please either provide me some test cases using jQuery or other custom syntax, or try running the tool against said syntax, and open issues (or otherwise let me know) for any problems that show up? Thanks!

Contributor

platinumazure commented Oct 7, 2015

@apsdehal The syntax tree transformation I selected, recast, was chosen specifically to try to preserve existing syntax and spacing and things, although it does have a few bugs when new statements are generated. Could you please either provide me some test cases using jQuery or other custom syntax, or try running the tool against said syntax, and open issues (or otherwise let me know) for any problems that show up? Thanks!

@apsdehal

This comment has been minimized.

Show comment
Hide comment
@apsdehal

apsdehal Oct 7, 2015

@platinumazure You can select data number 8, 9 and 10 from https://github.com/apsdehal/qunit-migrate/tree/master/tests/data. These all have been taken from jquery projects.

apsdehal commented Oct 7, 2015

@platinumazure You can select data number 8, 9 and 10 from https://github.com/apsdehal/qunit-migrate/tree/master/tests/data. These all have been taken from jquery projects.

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Oct 7, 2015

Contributor

Understood. I'll give it a try in the next couple of days.
On Oct 6, 2015 11:37 PM, "Amanpreet Singh" notifications@github.com wrote:

@platinumazure https://github.com/platinumazure You can select data
number 8, 9 and 10 from
https://github.com/apsdehal/qunit-migrate/tree/master/tests/data. These
all have been taken from jquery projects.


Reply to this email directly or view it on GitHub
#868 (comment).

Contributor

platinumazure commented Oct 7, 2015

Understood. I'll give it a try in the next couple of days.
On Oct 6, 2015 11:37 PM, "Amanpreet Singh" notifications@github.com wrote:

@platinumazure https://github.com/platinumazure You can select data
number 8, 9 and 10 from
https://github.com/apsdehal/qunit-migrate/tree/master/tests/data. These
all have been taken from jquery projects.


Reply to this email directly or view it on GitHub
#868 (comment).

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Oct 8, 2015

Member

It is great to see how this is going. Ping me if you need any help.

Member

leobalter commented Oct 8, 2015

It is great to see how this is going. Ping me if you need any help.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Oct 16, 2015

Member

How about we add links to both tools (while they're not merged, or whatever will happen) to the plugins page and the top of the migration guide? With those in place we can close this ticket, then update the links later as things progress.

Member

jzaefferer commented Oct 16, 2015

How about we add links to both tools (while they're not merged, or whatever will happen) to the plugins page and the top of the migration guide? With those in place we can close this ticket, then update the links later as things progress.

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Nov 4, 2015

Contributor

@jzaefferer 👍

@apsdehal Sorry for not getting back to you-- I've been incredibly busy at work. I'm hoping to try my tool out on your test cases this week.

Contributor

platinumazure commented Nov 4, 2015

@jzaefferer 👍

@apsdehal Sorry for not getting back to you-- I've been incredibly busy at work. I'm hoping to try my tool out on your test cases this week.

@Krinkle Krinkle added the task label Nov 4, 2015

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Apr 20, 2016

Member

@apsdehal @platinumazure was there any progress towards merging these tools?

Member

jzaefferer commented Apr 20, 2016

@apsdehal @platinumazure was there any progress towards merging these tools?

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Apr 20, 2016

Contributor

No, and I've stopped working on my tool. @apsdehal has made good progress
on his tool. It can be the "official" one, if everyone agrees it is up to
snuff.

Sorry, I just don't have the bandwidth to maintain my migration tool on top
of my other projects right now 😢
On Apr 20, 2016 4:40 PM, "Jörn Zaefferer" notifications@github.com wrote:

@apsdehal https://github.com/apsdehal @platinumazure
https://github.com/platinumazure was there any progress towards merging
these tools?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#868 (comment)

Contributor

platinumazure commented Apr 20, 2016

No, and I've stopped working on my tool. @apsdehal has made good progress
on his tool. It can be the "official" one, if everyone agrees it is up to
snuff.

Sorry, I just don't have the bandwidth to maintain my migration tool on top
of my other projects right now 😢
On Apr 20, 2016 4:40 PM, "Jörn Zaefferer" notifications@github.com wrote:

@apsdehal https://github.com/apsdehal @platinumazure
https://github.com/platinumazure was there any progress towards merging
these tools?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#868 (comment)

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Apr 20, 2016

Member

Alright, thanks for letting us know.

I lost track of the previous discussion (and I'm not sure what might still be relevant). Seems like @apsdehal could maybe add your test cases to this project. Either way, we should link to it in the migration guide.

Member

jzaefferer commented Apr 20, 2016

Alright, thanks for letting us know.

I lost track of the previous discussion (and I'm not sure what might still be relevant). Seems like @apsdehal could maybe add your test cases to this project. Either way, we should link to it in the migration guide.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Apr 21, 2016

Member

See qunitjs/qunitjs.com#120 for adding the note to the migration guide.

Member

jzaefferer commented Apr 21, 2016

See qunitjs/qunitjs.com#120 for adding the note to the migration guide.

@jzaefferer jzaefferer closed this Apr 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment