-
Notifications
You must be signed in to change notification settings - Fork 345
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
Bump OMD to 2.0.0~alpha1 #1321
Comments
Hi @patricoferris I haven't taken any issues yet to solve, do you think this would be a good start? 😃 |
HI @CarolVilarino I'm not sure this is a good first issue -- there's quite a lot of OCaml code to change and this repository doesn't have the |
@patricoferris can I try working on this? |
Hi @Srinithyee I've done a little extra digging on this issue, so let me explain some things and then you can see if you still want to work on it. The latest However, we can still work the the In summary, the work done on this won't be merged probably any time soon but it is still a contribution I believe you can write about in your Outreachy app :)) Let me know what you think, thanks. |
Just seen you are working on ocaml/ocaml.org#1492 -- it's best that you focus on that one I think. If anyone else wants to work on this given the above let me know :)) |
@Goodiec Since you were asking for a more challenging issue, you can take this one! :) |
@patricoferris Yes, I'd like to work on #1492. Thanks a lot for your clarification on this issue though :)) |
@Goodiec Are you still working on this issue? |
Hi @jyotibalodhi, yes I am. |
@Goodiec have you been able to make any progress? Do you need any help? Thanks! |
No worries @Goodiec. Did you read the first comment on the issue here: ocaml/ocaml.org#1321 (comment) :)
|
Yes, I did. What I gathered from the issue description is that I have to make some syntax changes to match the latest omd library in the affected places in the scripts but I wasn't sure. I will open a pull request on this soon and get validation that I am doing the right thing. Thank you @gs0510. |
Yes that sounds about right :) Thanks @Goodiec! :) |
Hello @patricoferris, @gs0510 . After installing the new Omd version, I went ahead to make some changes based on the example given above but trying to build the site using make local gives me the error in the image below: Please advise on this. |
Hi @Goodiec, awesome! I've just been talking about this work as the Omd team look to do a new major release. Absolutely no pressure, but just letting you know your work will be super useful ! This is a compiler error, in particular a type error. You have an print_endline (if true_or_false then "Hello" else "World")
Ignoring the I would recommend having a look at the old Omd interface and trying to understand how things map to the latest version. It is also important to work out what each function is trying to do and map that to the new version of Omd. So for this function that is causing the type error, the code is trying to highlight some OCaml code and return the generated HTML which is the Please be aware that this is not an easy issue at all, migrating to the new AST is not straightforward so please let me know if you encounter any other uncertainties. |
Thank you @patricoferris, I will go through the resources you provided and let you know if I encounter any difficulty or have further questions. |
Hello @patricoferris, I have been getting more familiar with Ocaml to help me better understand the functions I am to make changes to. Please could you provide me with a resource or resources that could help me understand the Omd interfaces better as I am still having blockers and there is no proper documentation on the library, I really want to work on this issue. |
As with most OCaml libraries the documentation is sparse. The best bet is to get comfortable with reading utop # Omd.of_string "# Hello World";;
- : Omd.doc =
[{Omd.bl_desc =
Omd.Heading (1, {Omd.il_desc = Omd.Text "Hello World"; il_attributes = []});
bl_attributes = []}] Unfortunately that's the nature of programming in OCaml, as you get more and more familiar with common practices the |
This issue is to upgrade the OCaml scripts which use Omd to the latest version of Omd. This issue is not necessarily very straight forward, but would expose someone to a lot of OCaml code, compiler errors and working in harmony with the type checker.
The main change is in the omd abstract syntax tree (AST) -- for example in
scripts/code.ml
we have:But the AST has changed, and
Omd.of_string
now returns adoc
type (https://github.com/ocaml/omd/blob/master/src/omd.mli#L66) so this pattern-matching with the latest Omd returns an error.So we need to convert instances like this to the new AST. For most cases this isn't too bad because the new AST simply wraps most things in a
block
type which has ablock_desc
and someattributes
. The fix for this line becomes:To start developing a solution be sure to update your version of omd i.e.
opam install omd.2.0.0~alpha1
.The text was updated successfully, but these errors were encountered: