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

Cleaning up template.distillery files (for 1.4 release) #539

Open
RogerTarani opened this issue Jan 14, 2019 · 0 comments
Open

Cleaning up template.distillery files (for 1.4 release) #539

RogerTarani opened this issue Jan 14, 2019 · 0 comments

Comments

@RogerTarani
Copy link

1/ In Makefile.os there are 10 times -ppx -ppx.
Is it normal?
As a test, replacing them by -ppx had no effect on make byte (but had effect on make opt that failed, as it seems to be useful to eliomopt. Anyway it's not blocking)

2/ Replacing deprecated Eliom_content.Html.F.pcdata with Eliom_content.Html.F.txt saved about 700 warning lines when doing make byte (out of about 1200 lines).

List of deprecated modules or functions:
Eliom_content.Html.F.pcdata -> JEliom_content.Html.F.txt
Js -> Js_of_ocaml.Js
Dom_html -> Js_of_ocaml.Dom_html
Dom -> Js_of_ocaml.Dom
Firebug -> Js_of_ocaml.Firebug

Can you update the ocsigen-start *.eliom files in template.distillery/ for the next 1.3.1 release?
It will make the build much less verbose.

You may do it with a sed command much more quickly than handling a PR.
I used the following command for example for Js -> Js_of_ocaml.Js :
$ sed -i 's/Js./Js_of_ocaml.Js./g' $(grep -l "Js." *.eliom)

3/ In demo_carousel1.eliom L33, the function To_dom.of_element belongs to the module Eliom_content.Html.
As I had a "Warning 33: unused open Eliom_content.Html." I deleted this open Eliom_content.Html directive, but then the compiler said Unbound module To_dom.
Tweaking theses directives, exchanging their order had no effect. At last, I just opened the module as a prefix to To_dom:

L34  let%lwt () = Ot_nodeready.nodeready (Eliom_content.Html.To_dom.of_element ~%carousel) in

Can you explain why this "unused open Eliom_content.Html" happened?

4/ In file "oc_1_3_0_test2_i18n.eliom" there are several Warning 27: unused variable f (4 lines).
(cause : line 147 and 852)

| En -> (fun ?(capitalize=false) ?(f=false) () -> List.flatten [[txt (if capitalize then "V" else "v")];[txt "alidated"]])
Inserting a null test worked:

L147 | En -> (fun ?(capitalize=false) ?(f=false) () -> List.flatten [[txt (if capitalize then "V" else "v")];[txt "alidated"];[txt (if f then "" else "")]])

L852 | En -> (fun ?(capitalize=false) ?(f=false) () -> String.concat "" [(if capitalize then "V" else "v");"alidated";(if f then "" else "")])

Does this null test still makes sense according to you?

Now I have a zero warning build.

@RogerTarani RogerTarani changed the title Cleaning up template.distillery files (Makefile and *.eliom deprecated functions or modules) Cleaning up template.distillery files (for 1.3.1 release) Jan 14, 2019
@RogerTarani RogerTarani changed the title Cleaning up template.distillery files (for 1.3.1 release) Cleaning up template.distillery files (for 1.4 release) Jan 14, 2019
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

No branches or pull requests

1 participant