Closed vkoves closed 5 years ago
@Watercycle - that definitely makes sense to me. I'm going to merge this now, but do you think it's worth decoupling that code, or will we just be starting from "scratch" with Vue?
@vkoves
My guess is that it will be quicker to work "from scratch" implementing Vue. Decoupling the data transformations and dom manipulation in its current JQuery state for all of the scheduler, in a dynamic language, with minimal tests, seems error prone. Plus, it will likely be easier to see how things should be split up while actively writing independent Vue components.
Since you wrote most of the scheduler and are more familiar with Vue, you're a better judge for that question. I'm biased towards saying the word "re-write" since I don't have as good of a grasp of the clientside code as you do.
I'm in agreement. The Vue code will be very different from our current code (and there'll be much less code, just from Vue handling stuff like data binding) so it really would be a complete rewrite.
That said, we'd also likely be writing a great deal of tests so we can ensure the new code is as solid as the old.
On Fri, Mar 1, 2019, 1:49 AM Matthew Spero notifications@github.com wrote:
@vkoves https://github.com/vkoves
My guess is that it will be quicker to work "from scratch" implementing Vue. Decoupling the data transformations and dom manipulation in its current JQuery state for all of the scheduler, in a dynamic language, with minimal tests, seems error prone. Plus, it will likely be easier to see how things should be split up while actively writing independent Vue components.
Since you wrote most of the scheduler and are more familiar with Vue, you're a better judge for that question. I'm biased towards saying the word "re-write" since I don't have as good of a grasp of the clientside code as you do.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vkoves/carpe/pull/399#issuecomment-468575349, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCjS65DLiKGeEn_6zu9QLS1MpX8_Il6ks5vSNuVgaJpZM4bTtpt .
Description
This PR starts the work for issue #344, splitting up
schedule.js
into a few distinct files. Unfortunately, it seems most of the schedule file is related to the scheduler functionality and can't really be separated out much more. However, I would love feedback to see what I could pull out further in another PR, or if I could refactor things more abstractly so it can be reused and pulled out.Note: This is up on Carpe test so you can confirm the scheduler is working properly. There aren't many tests, so it is a bit precarious. However, this PR is mostly cutting and pasting code, and going commit by commit should make that very clear.
Type of Pull Request
Based on the contributor's guide, this PR is of type:
feature-branch
->dev
)hotfix-branch
->master
)release-branch
->master
)Requestor Checklist
Requestor: Put an
x
in all that apply. You can check boxes after the PR has been made.Reviewer: If you see an item that is not checked that you believe should be, comment on that as part of your review.
How This Has Been Tested
Tested locally by confirming that the scheduler seems to be working properly.