Skip to content
Snippets Groups Projects

Minimal c interface

Merged Timm Fitschen requested to merge f-extern-c into dev

Summary

See https://gitlab.indiscale.com/caosdb/customers/lfpb/management/-/issues/404

Integration tests are in caosdb-cppinttest!1 (merged)

Focus

  • Please take a look at the c-interface functions:
    • I the naming consistent?
    • I see one problem regarding the destructors: When a client uses caosdb_connection_connection_manager_get_default_connection to get the default connection, the destructor caosdb_connection_delete_connection should not be called, because the connection object is actually owned by the ConnectionManager. Maybe, we should prevent the deletion of the wrapped connection somehow, when the connection is owned by the ConnectionManager?
  • ConfigurationManager and ConnectionManager:
    • These two singletons are tightly coupled. When the ConfigurationManager is being reset, also the ConnectionManager resets. Is this a good behavior?
  • There is currently no error/exception handling in the c-interface. I will add this in https://gitlab.indiscale.com/caosdb/customers/lfpb/management/-/issues/390

Test Environment

Please try and setup a client (e.g. the ccaosdbcli) with a json configuration file.

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

  • All automated tests pass
  • Reference related Issues
  • Up-to-date CHANGELOG.md
  • Annotations in code (Gitlab comments)
    • Intent of new code
    • Problems with old code
    • Why this implementation?

Check List for the Reviewer

  • I understand the intent of this MR
  • All automated tests pass
  • Up-to-date CHANGELOG.md
  • The test environment setup works and the intended behavior is reproducible in the test environment
  • In-code documentation and comments are up-to-date.
  • Check: Are there spezifications? Are they satisfied?

For further good practices have a look at our review guidelines.

Edited by Florian Spreckelsen

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Timm Fitschen
  • Timm Fitschen marked the checklist item All automated tests pass as completed

    marked the checklist item All automated tests pass as completed

  • Timm Fitschen marked the checklist item Reference related Issues as completed

    marked the checklist item Reference related Issues as completed

  • Timm Fitschen changed the description

    changed the description

  • Timm Fitschen marked the checklist item Annotations in code (Gitlab comments) as completed

    marked the checklist item Annotations in code (Gitlab comments) as completed

  • Timm Fitschen requested review from @florian

    requested review from @florian

  • mentioned in merge request caosdb-cppinttest!1 (merged)

  • Florian Spreckelsen changed the description

    changed the description

  • Florian Spreckelsen marked the checklist item I understand the intent of this MR as completed

    marked the checklist item I understand the intent of this MR as completed

  • Florian Spreckelsen marked the checklist item All automated tests pass as completed

    marked the checklist item All automated tests pass as completed

  • Florian Spreckelsen marked the checklist item Check: Are there spezifications? Are they satisfied? as completed

    marked the checklist item Check: Are there spezifications? Are they satisfied? as completed

  • added 1 commit

    • ada1225c - DOC: Fix a typo in README_SETUP

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading