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

Added get_bmi_version to sidl and documentation #111

Merged
merged 6 commits into from
Oct 10, 2022

Conversation

RolfHut
Copy link
Contributor

@RolfHut RolfHut commented Sep 21, 2022

solves #8

@mcflugen and I added get_bmi_version to the sidl and the documentation, but it still needs to be added to the different repos of the language specific bindings.

@mdpiper
Copy link
Member

mdpiper commented Sep 26, 2022

I'd like to touch the docs submitted with this PR, but otherwise it could be merged with lazy consensus.

@mdpiper
Copy link
Member

mdpiper commented Sep 26, 2022

The new get_bmi_version function is listed separately in the SIDL file, but under the model information functions in the docs. I'd like to propose modifying the SIDL file, moving the function def to match the docs like so:

    // Model information functions
    int get_component_name(out string name);
    int get_bmi_version(out string version);
    int get_input_item_count(out int count);
    ...

What do you think?

@RolfHut
Copy link
Contributor Author

RolfHut commented Sep 26, 2022

I'm ambivalent. @mcflugen and me had a short discussion on it: the thinking was (iirc) that the bmi_version does not say anything about the model, only on the implementation of bmi and that this could cause confusion. Will you update PR for this, or should I?

@mdpiper
Copy link
Member

mdpiper commented Sep 26, 2022

I'm ambivalent. @mcflugen and me had a short discussion on it: the thinking was (iirc) that the bmi_version does not say anything about the model, only on the implementation of bmi and that this could cause confusion. Will you update PR for this, or should I?

Ah, I see; that makes sense. If you'd prefer, I could invert my suggestion above and make a new section in the docs. I'd just like the docs to mirror the SIDL, and vice versa.

If you give me permissions to write to RolfHut:rolfhut/addBMIVersion, I can make changes.

@mdpiper
Copy link
Member

mdpiper commented Sep 26, 2022

@mcflugen What do you think--match the docs to the SIDL?

@mdpiper
Copy link
Member

mdpiper commented Sep 28, 2022

I moved the new get_bmi_version function to a new section in the docs, so it matches the ordering in the SIDL.

@RolfHut
Copy link
Contributor Author

RolfHut commented Sep 29, 2022

all ok with me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants