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

Ruby bindings gem #1946

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@BernhardDenner
Contributor

BernhardDenner commented Apr 28, 2018

Purpose

Make a Ruby gem for elektra bindings

State

Currently this is at an very early stage and needs additional work

  • do not use elektra private headers and API within the binding code (gems should be installable without the elektra sources)
  • create gem with mkmf build
  • integrate gem build into cmake build
  • version schema? elektra + gem version,
  • gem name? elektra is already in use -> libelektra ???
  • publish gem

Checklist

  • commit messages are fine ("module: short statement" syntax and refer to issues)
  • I added unit tests
  • I ran all tests locally and everything went fine
  • affected documentation is fixed
  • I added code comments, logging, and assertions (see doc/CODING.md)
  • meta data is updated (e.g. README.md of plugins)
  • release notes are updated (doc/news/_preparation_next_release.md)
@markus2330

This comment has been minimized.

Contributor

markus2330 commented Apr 28, 2018

Great to see progress here! Btw. @MasterToney has similar plans, some coordination would be useful!

@BernhardDenner

This comment has been minimized.

Contributor

BernhardDenner commented May 2, 2018

@MasterToney I also have also a gem boilerplate which is currently not part of this PR. Just for info ...

@BernhardDenner

This comment has been minimized.

Contributor

BernhardDenner commented May 2, 2018

gem name: how should we call the gem? elektra is already taken, options

  • libelektra
  • kdb
  • elektra-kdb
  • ...?
@BernhardDenner

This comment has been minimized.

Contributor

BernhardDenner commented May 2, 2018

👍 libelektra 😃

@markus2330

This comment has been minimized.

Contributor

markus2330 commented May 2, 2018

Yes, I also favor libelektra. 👍

@markus2330

This comment has been minimized.

Contributor

markus2330 commented May 8, 2018

Should we merge this for 0.8.23?

Would be great if we were able to build a gem for 0.8.23.

@@ -47,8 +47,10 @@ namespace std {
%{
extern "C" {
#include "kdbconfig.h"
#include "kdbprivate.h" /* required for KEYSET_SIZE */
#include "kdblogger.h"
/* #include "kdbprivate.h" required for KEYSET_SIZE */

This comment has been minimized.

@markus2330

markus2330 Jun 5, 2018

Contributor

Why not simply always pass RARRAY_LEN(argv[0]) to the constructor? The ksNew implementation already makes sure that at least KEYSET_SIZE elements are allocated.

Then KEYSET_SIZE would not be needed.

Otherwise I think this is ready to go. Could you please rebase, add something to the release notes, and add "ready to merge"?

Thank you for working on this!

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