-
Notifications
You must be signed in to change notification settings - Fork 295
Fix #205 (erlydtl:compile/3 returns warnings) #210
Conversation
Adapt rebar_erlydtl_compiler to handle warnings emitted by erlydtl:compile/3.
Erlydtl cannot write file if found warnings. Please see my comment in #205. |
So, erlydtl:compile/3 by default does not succeed if there are warnings? |
Yes, it will not generate the beam file. |
This bug has been fixed in erlydtl 0.8.0. In 0.7.0, see https://github.com/erlydtl/erlydtl/blob/0.7.0/src/erlydtl_compiler.erl#L99 in 0.8.0, see https://github.com/erlydtl/erlydtl/blob/0.8.0/src/erlydtl_compiler.erl#L88 |
Should we merge #210 then? Any erlydtl backward compatibility issue this would cause? |
If using erlydtl 0.7.0, the beam file cannot be generated. You can consider #211 as a solution. |
But wouldn't that report warnings one way via erlydtl and errors another way via rebar? I'm not sure that's a good idea. What about detecting >= 0.8.0 at runtime? |
I think it's the only solution. |
OK, do you want to submit a (new) patch with that solution? |
Yes, my pleasure. |
Reopen. Once erlydtl/erlydtl#116 is resolved, we should merge this patch. |
@@ -210,6 +210,8 @@ do_compile(Config, Source, Target, DtlOpts) -> | |||
Opts) of | |||
ok -> | |||
ok; | |||
{ok, _Mod, _Bin, Ws} -> | |||
rebar_base_compiler:ok_tuple(Config, Source, Ws); | |||
error -> |
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.
@Tuncer erlydtl/erlydtl#116 adds binary option.
If there is no option {binary, true}
in erlydtl_opts
, erlydtl will return tuple {ok, Mod, Ws}
instead of {ok, Mod, Bin, Ws}
.
So this PR should add {ok, Mod, Ws}
clause.
Please see https://github.com/goofansu/rebar/commit/da9b08228fb71bee4a74b20cf627f7f5b2a05a38
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.
Thanks, fixed!
erlydtl implemented 'binary' option. Add new clause for {binary, false}.
👍 This looks good. |
Looks good to me too, this ready @Tuncer? |
@jaredmorrow yes. |
Fix #205 (erlydtl:compile/3 returns warnings)
Adapt rebar_erlydtl_compiler to handle warnings emitted by erlydtl:compile/3.