Skip to content
Snippets Groups Projects

Minimal c interface

Merged Timm Fitschen requested to merge f-extern-c into dev
All threads resolved!

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

Pipeline #10589 passed

Pipeline: caosdb-cppinttest

#10590

    Pipeline passed for 94a49b8b on f-extern-c

    Approval is optional

    Merged by Timm FitschenTimm Fitschen 3 years ago (Jul 21, 2021 10:17am UTC)

    Merge details

    • Changes merged into dev with 0bf92e25.
    • Deleted the source branch.

    Pipeline #10593 passed

    Pipeline: caosdb-cppinttest

    #10594

      Pipeline passed for 0bf92e25 on dev

      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

    • added 1 commit

      Compare with previous version

    • Florian Spreckelsen marked the checklist item The test environment setup works and the intended behavior is as completed

      marked the checklist item The test environment setup works and the intended behavior is as completed

    • Florian Spreckelsen resolved all threads

      resolved all threads

    • Florian Spreckelsen approved this merge request

      approved this merge request

    • Timm Fitschen added 1 commit

      added 1 commit

      • 94a49b8b - DOC: add EXPORT USE ONLY hints to some c-interface functions

      Compare with previous version

    • Timm Fitschen mentioned in commit 0bf92e25

      mentioned in commit 0bf92e25

    • merged

    • Please register or sign in to reply
      Loading