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

Merge with shymega/guile-rs #8

Open
shymega opened this issue Mar 18, 2018 · 7 comments
Open

Merge with shymega/guile-rs #8

shymega opened this issue Mar 18, 2018 · 7 comments
Milestone

Comments

@shymega
Copy link
Collaborator

shymega commented Mar 18, 2018

As we've discussed over email and over IRC on Mozilla/#guile-rs, this issue is to track the progress of merging this project with my guile-rs repository.

I have reached out to contributors of my guile-rs repository to get their permission for their contributions to be included into the new merged project of Rust bindings to GNU Guile.

@shymega
Copy link
Collaborator Author

shymega commented Mar 18, 2018

I've heard back from the two other contributors to my version of guile-rs, who have given their permission for their code to be relicensed.

That has been approved, so in terms of code licensing, there are no issues with contributions.

@Javyre
Copy link
Contributor

Javyre commented Mar 18, 2018

Sounds great!
Ill add you as a contributor, you can create a new branch for the merge.
Ill be glad to help out.

(mentioning @cherryman for the licensing...)
(edit: you should have write permissions to the repo now)

@shymega
Copy link
Collaborator Author

shymega commented Mar 18, 2018

@Javyre Sure, I've got the invitation, accepted it.

I can create a branch, I'm not too sure how to begin the merging, and I don't know how you and @cherryman wanted it to be structured. What should we begin with?

Cheers.

@cherryman
Copy link
Member

cherryman commented Mar 19, 2018

There are some structures that are shared between both projects (namely the Guile[VM] and Scm types) which seem to be very similar, so they could easily be merged.

API changes

Guile

The following could be used from @shymega's branch

  • shell method from GuileVM
  • undefined_variable from GuileVM (up for discussion)

The define and define_subr1 calls are being implemented as macros in this repository, which allows for any number of arguments as well as optional arguments.

Scm

Many methods have already been implemented. There are some missing, which could be useful

  • force_cast
  • is_unsigned_integer and is_signed_integer (Would probably be done using Scm<Integer>

Encode/Decode

The remaining methods for the Scm type are check_type and case, which use the Encode/Decode system. @Javyre expressed a strong interest in it. Merging would require changing the way this branch does type conversion and use the Encoder system instead, which IMO is much nicer. There aren't too many methods for conversion, and some of them have been done already.

For example, see the string conversions in this branch in src/scm/string.rs and in @shymega's branch see src/repr.rs. I find that having a separate repr module is better since a separate module is used to define conversions between rust and guile types, however this is up for discussion.

Organisation

We could either track the changes here, since there aren't many, or make a milestone with multiple sub-issues. Since the merging isn't too complex, and will take place in a separate branch, I don't think we'll need an extra milestone.

For the merging itself, I don't have many ideas. One is to import as a git submodule and incrementally move over relevant sections. I'm sure there have been similar moves in the past by other projects, so if we find something similar then we could potentially use it as inspiration.

And finally, the biggest problem: what do we name the branch used for merging?

(@Javyre edited: minor corrections)

@Javyre Javyre added this to the 1.0.0 Release milestone Mar 25, 2018
@shymega
Copy link
Collaborator Author

shymega commented May 18, 2018

Sorry, just got back to taking a look at this issue.

Just wondering, what Rust channel are we targeting? Nightly? Stable?

I've created a branch on this project for me to work on. We also need to think about how to redirect people from my guile-rs repo to your one. I think it makes more sense to be associated with your project. I suppose a warning in the README would suffice!

@shymega
Copy link
Collaborator Author

shymega commented May 18, 2018

Oh, also: I see a lot of build-deps for your version of guile-rs. Could we lose any dependencies? Probably not a bad idea to keep things simple :-)

@Javyre
Copy link
Contributor

Javyre commented May 21, 2018

I'm always open to minimize dependencies
But I'd to preserve current functionality as well...

IIRC, most of the dependencies come from the build script.. we could pre-generate the code to minimize these deps

We are targetting the stable channel.

I've been pretty busy with school myself (as well as @cherryman ), I should be back on task in about a month on this project

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

No branches or pull requests

3 participants