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

CP-26717: Update Xapi to use PPX-based Gpumon IDL #3418

Merged
merged 3 commits into from
Feb 2, 2018

Conversation

minishrink
Copy link
Contributor

@minishrink minishrink commented Jan 28, 2018

This PR is the second of three to port GPU monitoring daemon gpumon to use PPX-based code generation for RPCs.

  1. Port Gpumon interface
  2. this
  3. Port Gpumon

| _::_ -> failwith @@ Printf.sprintf
"%s: VM %s (dom %d) has more than one NVIDIA vGPU (%s)"
this (Ref.string_of vm) domid __LOC__
|> fun self -> Db.PCI.get_pci_id ~__context ~self
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation here has gone wild as well... it would be nice to see if there is a flag to stop this behaviour in ocp-indent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it assumes it's part of the same function, which would make sense. Not sure how you'd differentiate between that without parentheses/begin-end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the use of the |>

Copy link
Contributor Author

@minishrink minishrink Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what if the code above was like this:

|> (function
      | x -> do_this
      | y -> some_function
            |> followed_by_this_one)
|> something_with_result_of_pattern_match

I'm not sure how it would parse that without the parentheses

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect to use parentheses in that case in fact

Copy link
Contributor Author

@minishrink minishrink Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's currently the situation; the original function wasn't wrapped in parentheses, so ocp-indent assumed the next |> function was part of the same lambda function, hence the weird indent. I suppose we could still try to decrease the size, since it doesn't need to be that far in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd like to understand if it is right or not :)

@@ -152,7 +151,7 @@ module Nvidia = struct
let vgpu_metadata () =
Db.VGPU.get_compatibility_metadata ~__context ~self:vgpu
|> fun md -> [List.assoc key md]
|> List.map Stdext.Base64.decode in
|> List.map Stdext.Base64.decode in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird indent

Akanksha Mathur added 3 commits January 30, 2018 20:59
Signed-off-by: Akanksha Mathur <akanksha.mathur@citrix.com>
Signed-off-by: Akanksha Mathur <akanksha.mathur@citrix.com>
Signed-off-by: Akanksha Mathur <akanksha.mathur@citrix.com>
@mseri mseri merged commit 2a78d9d into xapi-project:master Feb 2, 2018
@minishrink minishrink deleted the gpu_ppx branch February 15, 2018 15:40
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

2 participants