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

Support middleman v4.1 #12

Merged
merged 12 commits into from Sep 4, 2016
Merged

Conversation

ymph
Copy link
Collaborator

@ymph ymph commented Jul 21, 2016

Hello,

First thank you for this extension.
It is quite useful.
I made some changes to make it work with middleman 4.1 (4.1.10 right now)
Here are the modifications:

  • I merged the pull request Support Middleman v4 #11 from philiph/support-middleman-v4 (thank you @philiph)
  • After (too) much debugging (and trial and errors), I determined that the reason the extension was not working in middleman 4.1 was the introduction of the parallel mode (c.f. https://github.com/middleman/middleman/blob/master/CHANGELOG.md#410). In parallel mode, the extension helpers are run in another process and therefore do not share the list of image to create in the "after_build" callback. So I devised this hack to use a temporary file (with read/write lock) to store the list of image. This is quite ugly and vastly inefficient, but seems to get the job done

Please tell me what you think, I am really eager to know your take on this issue and would like to see a better solution than the one I used.

Thank you,

Regards,

Yves-Marie Haussonne

@kubenstein
Copy link
Owner

hi @ymph, im so surprised and happy to see someone uses and wants to contribute. To be honest i switched stacks and i dont use middleman very often these days, thats why this repo is a bit rusty. Im really impressed you debugged and fix code and tests for version 4. really good job!
i think your solution with locked file is ok, if the problem is sharing resourced between threads we have to introduce some sharing mechanism. i would just a bit tweak key methods, so store_resized_image and after_build to hide details about sharing implementation and thus increase readability what actually is going on there - we save and read resized_images to shared storage. Maybe wrap code with method with block that internally write and read from file. we can collaborate.
About efficiency, luckily, we dont have to worry that much. during development i think your change wont drastically affect performance and during build, well, good to have stuff fast but building is one time operation not 100x per second so we can accept it but did you notice any drop in performance? i think it should be fine, version 3 of middleman wasnt using any parallelism and it was fast enough not to annoy at least in my projects.

Thx once again for all your time and effort!
Kuba

@ymph
Copy link
Collaborator Author

ymph commented Jul 27, 2016

Hello,

First for information, and if this is ok for you to say, what stack are you using now ?

Next, I just pushed something that hide the implementation details at the extension(.rb) level.
It is what you meant in your message ?

About efficiency, I also is not really concerned in my project. I use this extension on a very little static site. It would certainly have been a lot more time efficient to create the resized image manually, than to hack this extension ! But when you start to scratch this itch...

But in fact, what is "bothering" me the most about the future of this extension is the fact that it doesn't really fit what looks like the "normal" extension lifecycle. This extension is creating new files. If we are rigorous, I think that these files should belong to the sitemap and the extension should add them to it. The problem is that the sitemap is established by middleman (and by delegation by the extensions) before the file emission phase, whereas this extension will have its file list after the file emission. There is a mismatch here.

For inspiration, I have tried to see if another extension is operating with the same principle. I have looked in the "official" Middleman Extension Directory (https://directory.middlemanapp.com), but I could not fond another one...

I do not know exactly how (question on a forum, opening a github ticket,...), but I think I will try to contact the Middleman team on this subject and ask them what is their stance on this, and whether or not it would be possible to improve the situation. In any case, the Middleman documentation on extension need to be more completed I think.

Thank you,

Regards,

ymh

PS: I decided to make a post on the middleman forum : Middleman 4.1 and extension lifecycle

@kubenstein
Copy link
Owner

hi, thank you for all effort, i think extracting to separate class is really nice move, now all those locks and stuff are nicely hidden. I have to say i also were struggling with life cycle and the fact that the extension doesn't fit to middleman philosophy. Usually other extensions create files based on existing static files and static configuration, in our case information how to generate thumbnail is in the view so we have to build views first and that messes up everything. I also were experimenting to push all thru sprockets but sprockets is such a complicated tool that i wasnt able to do it in reasonably clean way.


Answering your question about stack, i moved to angular first and then to react. i built couple of sites on middleman to have all scss and minifications goodies, with angular frontend. my personal website is one of example. but i found a bit cumbersome to use ruby for managing js technologies so for most of my frontend stuff i switched to pure js builders, gulp at first for angular and then webpack for react stuff. i like to have api with data, for static pages its often static json file. but its easy to bind real server if project evolved and managing data needs to be more dynamic. For backend i still find ruby way cleaner and fun to code in.


So as you see i dont use this extension any more. You are really passionate and engaged person, if you want to overtake the project i would love to give it to good hands.

Kuba

@ymph
Copy link
Collaborator Author

ymph commented Aug 8, 2016

Hello,

Thank you for the information on your new stack, it is always interesting to observe the technical choices of others.

Thank you for your kind words, I would be glad and honoured to help and make this extension live, so proceed as you see fit.

Regards,

ymh

@kubenstein
Copy link
Owner

hey hey, sorry for late reply, it would be really nice if you can overtake the project. i wonder how to do it tho. on github i guess you just keep your fork and ill put in readme that your repo is now official. The thing im not sure is how to transfer gem ownership on rubygems. do you have rubygems account?

@ymph ymph mentioned this pull request Aug 25, 2016
@ymph ymph merged commit b7ead86 into kubenstein:master Sep 4, 2016
@ymph ymph deleted the support-middleman-v4.1 branch September 4, 2016 07:59
@ymph ymph mentioned this pull request Sep 4, 2016
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