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

Add a public API specification #327

Merged
merged 6 commits into from
Mar 8, 2016
Merged

Conversation

dwlehman
Copy link
Contributor

@dwlehman dwlehman commented Mar 2, 2016

Where a class is listed that means instantiating that class is supported. Where is it not, we are specifying the public methods and attributes. There are some issues with undocumented members and singletons, but they are minor.

@vpodzime
Copy link
Contributor

vpodzime commented Mar 3, 2016

So if for example ~blivet.devices.lvm.LVMLogicalVolumeDevice is listed, it means that all its methods, attributes and properties are part of the API?

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 3, 2016

It means the constructor, any listed methods/attributes, and all methods/attributes listed in superclasses are public.

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 3, 2016

Would it be clearer (if a bit verbose) to explicitly list everything for each class?

@vpodzime
Copy link
Contributor

vpodzime commented Mar 3, 2016

Well, what if I want one or two methods/attributes not being part of the API?

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 3, 2016

Then you don't list them in the API spec.

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 3, 2016

By "listed" I mean "listed in the API spec".

@vpodzime
Copy link
Contributor

vpodzime commented Mar 3, 2016

Then you don't list them in the API spec.

Sure, do I need to list all the other methods/attributes then?

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 3, 2016

Yes, and this makes it clear that I should convert it so that every method/attribute is explicitly listed directly in the list for that class instead of expecting users to check the superclass' list.

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 4, 2016

New version has several changes:

  • the public API specification is more explicit
  • the TOC now only shows top-level items: Introduction, Public API, Testing, &c
  • the existing singleton modules were cleaned up a bit:
    • instance and class names don't match
    • class names are public so they get documented
  • direct access to full module docs from the streamlined TOC
    • old: blivet->blivet package
    • new: blivet package

@vpodzime please check to see if I missed anything in the new LVM classes.

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 7, 2016

Isn't map_name equal to name for DMDevice (including lvs)?

What about is_thin_lv, is_thin_pool, is_snapshot_lv?

Sphinx' mocking doesn't work for us since the objects aren't
subscriptable, which is a problem for the parted fileSystemType
attributes in format classes. We haven't had working docs on RTD
for a long time anyway, so just drop all of this crud for now.
@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 7, 2016

Latest version includes the following changes:

  • fixed a typo in the singleton cleanup patch
  • added more to lvm device API
  • drop sphinx mocking entirely (it wasn't working)

@dwlehman
Copy link
Contributor Author

dwlehman commented Mar 7, 2016

You can see a preview of the docs here.

@vpodzime
Copy link
Contributor

vpodzime commented Mar 8, 2016

Isn't map_name equal to name for DMDevice (including lvs)?

It is, right now, but will users of the API know? And is it going to stay like that in the future? I imagine us using e.g. vg_name/lv_name for LV name.

What about is_thin_lv, is_thin_pool, is_snapshot_lv?

Good catch! These too, plus is_internal_lv.

@vpodzime
Copy link
Contributor

vpodzime commented Mar 8, 2016

Looks good to me otherwise.

dwlehman added a commit that referenced this pull request Mar 8, 2016
Add a public API specification
@dwlehman dwlehman merged commit c4dc23c into storaged-project:master Mar 8, 2016
@dwlehman dwlehman deleted the api-spec branch March 8, 2016 16:36
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.

None yet

2 participants