Skip to content

Remove all parametrized modules for R16 #513

Closed
mworrell opened this Issue Jan 31, 2013 · 21 comments

4 participants

@mworrell
Zotonic member
  • Needs upgrade of MochiWeb.
  • Cleanup of some parts of WebZmachine.

Any other parametrized modules around?

@mmzeeman
Zotonic member

Erlydtl can do parameterized module variable lookups, but I see that the erlydtl compiler does not use them. So I guess that doesn't count.

@mworrell
Zotonic member

We just remove that from the runtime :+1:

@mworrell
Zotonic member

Need to investigate what the update to MochiWeb is - if it removes parametrized modules or adds some parse transforms as an intermediate solution.

@mmzeeman
Zotonic member

For webzmachine I see a wm_resource.hrl on the horizon... and a couple of small rewrites... Not too bad. Or do we call it wm_controller.hrl to bring it more in line with zotonic's naming conventions.

And for mochiweb... oops... that is quite some work...

You mentioned moving to either cowboy or https://github.com/knutin/elli Maybe that moving to a different base http server is a better option if we have to invest a considerable amount of time into this.

Looking through their repo's their github repo's I kind of like the minimal approach of elli. I have the gut feeling that webzmachine + elli would mix very well. But that is just based on gut feeling and nothing else. At least elli knows that one SHOULD use properly capitalized http headers. :-)

@kaos
Zotonic member
kaos commented Jan 31, 2013

+1 for elli, also no real know-why.. only looked through the code, and liked what I saw :)

@mworrell
Zotonic member
mworrell commented Mar 8, 2013

Cleaned up webzmachine.

See zotonic/webzmachine@cafba55

Now, who will do the elli thing....

@kaos kaos pushed a commit to zotonic/webzmachine that referenced this issue Mar 11, 2013
@mworrell mworrell Removed last webzmachine specific parametrized module. cafba55
@arjan
Zotonic member
arjan commented Mar 14, 2013

Created new ticket for this, #533

@arjan arjan closed this Mar 14, 2013
@kaos
Zotonic member
kaos commented Mar 15, 2013

Reopening, since this issue is about getting rid of the parametrized modules, and not about replacing mochiweb with elli.

@kaos kaos reopened this Mar 15, 2013
@mworrell
Zotonic member

:+1: We can close this one when we are running on R16B

@mworrell
Zotonic member

I propose to (for now) add the parametrized-module-parse-transform to MochiWeb.

@kaos
Zotonic member
kaos commented Mar 15, 2013

I don't think they use the parse transform.. see this commit mochi/mochiweb@23a1d48

I would like to pull their changes into our zotonic/mochiweb.

@mworrell
Zotonic member
@kaos
Zotonic member
kaos commented Mar 15, 2013

:+1:

@mworrell
Zotonic member

Merged branch mochi/mochiweb@f3f133b from mochiweb in our version.

Our changes (compared to origin) amount to:

  • mochiweb_html additions/changes
  • we support separate any/any6 options
  • any6 listens to ipv6 with extra options
  • added SSL timeouts
@kaos
Zotonic member
kaos commented Mar 18, 2013

cool :+1:

@arjan
Zotonic member
arjan commented Mar 18, 2013

what about making pull requests upstream for these changes?
It seems they are pretty universal.

@mworrell
Zotonic member

We will need some shaving on the any/any6 differences, and testing with the socket options.
Other than that, they seem to be quite ready to be merged upstream.

@mworrell
Zotonic member

Will make separate issue in zotonic/mochiweb for that.

@mworrell mworrell referenced this issue in zotonic/mochiweb Mar 18, 2013
Open

Merge our changes upstream #2

@mworrell
Zotonic member

Back ref: zotonic/mochiweb#2

@mworrell
Zotonic member

I am not aware of any other parametrized modules in Zotonic.
Closing this issue (reopen if we find some other parametrized modules)

@mworrell mworrell closed this Mar 18, 2013
@mworrell mworrell added a commit that referenced this issue Sep 22, 2013
@mworrell mworrell core: remove some more parametrized module references. Issue #513
(cherry picked from commit 99325a1)
67a3d19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.