-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Refactored blaze to move ES6 version #397
base: release-3.0
Are you sure you want to change the base?
Conversation
Thanks a lot @klablink! Can you please sign the CLA, so that we can proceed on this? |
Hi @StorytellerCZ, I have signed the CLA now. |
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.
There are some things to resolve, especially the Es5 to ES6 classes and the disabled eslint warnings. Let me know if you need help at any point.
I have completed the migration of all packages, tell me if there is anything to change. |
@StorytellerCZ @jankapunkt The removal of the UI package makes the iron-router package incompatible. I cannot find it in the community-managed packages, so in my opinion it should be added. What do you think? |
This is a sign that iron router has to be updated and that there is lots of legacy code in it and maybe even a few parts that could be vulnerable (afaik iron router can be used on the server, too, right!?) |
Right, I think many projects started with Meteor in version 0.x still use this package. We use it more client-side, server-side we use express. |
@klablink can you please split this PR into smaller ones by packages, so that we can review this more efficiently. Thank you! |
Hi @StorytellerCZ I can try but it is not easy because some changes are across packages. |
@StorytellerCZ I checked the possibility of splitting PR. Unfortunately, the packages are strongly interconnected, the modification of a construct implies the revision of all references. For example, the transformation of Blaze objects into ES6 classes involves the use of the new clause in the linked sources. Now, how can we proceed to simplify the verification task? |
Alternatively we could split reviews between @StorytellerCZ , somebody else and me? |
I think we need to split lint fixes from this PR. There area lot of files where the only change is that it has been linted. That should go into a separate PR, we merge that and that should reduce the number of files that need to be reviewed. |
Yes @StorytellerCZ good point. The biggest changes, apart from linting, are:
the rest is mostly linting. |
Lets split this up, and get this in asap. |
As requested in #367, I started to migrate the code to ES6 for the blaze package.
All tests are passing, please review and let me know for required changes.