search

fbonetti / elm-rails

View helpers for incorporating Elm modules into Rails views
MIT License
75 stars 20 forks source link

Fix nested dependency tracking #24

Closed botandrose closed 7 years ago

botandrose commented 7 years ago

Howdy, Frank! I'm back with another patch... this time fixing nested dependency tracking.

As it stands, if you have a Elm file that imports an Elm file that imports an Elm file, changes to the third file will not cause a recompilation in Sprockets, neither while developing locally, nor while performing deployment compilation on production. Turns out the dependency tracking was broken. While the comments said it should recurse, it didn't actually do so.

The fix was easy enough, but I also took this opportunity to set up unit tests alongside the existing acceptance tests, so that this fix could be verified with a test, and refactored the main class a bit.

I reckon this is biting anyone with a non-trivial Elm app. WDYT?

botandrose commented 7 years ago

Herro, prease? Just want to ping, and note that this has been working great in production for the last two months or so.

fbonetti commented 7 years ago

Hey Micah,

I don't have time to maintain my open source libraries and I've largely handed over ownership to other people. I've also never personally used this Gem and don't follow it's development very closely.

I invited you to be a collaborator on this project. You might want to get in touch with Louis Simoneau https://github.com/lsimoneau before merging anything, since he's the main contributor to this project, but I'll leave it up to you to decide how to move forward.

On Mon, Jul 10, 2017 at 5:59 PM, Micah Geisel notifications@github.com wrote:

Herro, prease? Just want to ping, and note that this has been working great in production for the last two months or so.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fbonetti/elm-rails/pull/24#issuecomment-314273683, or mute the thread https://github.com/notifications/unsubscribe-auth/ACI_Aglj8vutSxb1q7fAywg1axgjf7Lmks5sMqzqgaJpZM4NptnG .

botandrose commented 7 years ago

Hey Frank, thanks for adding me to the project! I am using my fork of this in production on several projects, so it will be nice to be able to influence its development more directly on mainline.

@lsimoneau I'll PM you regarding collaboration on this repo.

lsimoneau commented 7 years ago

My apologies, I read this code when you pinged the issue 2 days ago and meant to merge but something must have come up 😞 .

botandrose commented 7 years ago

:tada: