-
Notifications
You must be signed in to change notification settings - Fork 19
Discuss merging stellar-ruby-sdk and stellar-ruby-base #58
Comments
I think this point is moot because
We probably should have put the SEP-10 helpers in All in all I'm 👍 with merging them. |
Thanks for the mention. +1 to merging them. As a user, the main inconvenience has been needing to grep around in 2 different repos to find things. |
The Python Stellar SDK has always had only one GitHub repository, but it has two PyPi package names. In the early stage, it was called Sorry, I just accidentally replied to this issue with my work account, and then I deleted it. 😅 |
I'm in favour also. There's a potential benefit if someone was going to use a pure tx construction library without the client, but that doesn't seem likely here, as the concerns are muddled. 👍 to combining them for simplicity. |
Should we move the base code into the SDK project? This way seems like it would be the least likely to cause issues, compared to moving the SDK into the base project. |
I'm in favor. The separation was always a bit unclear. 👍 for base into sdk and not the other way around. Let's be very careful about this process so that if someone relies on base directly they get ample warning. |
According to rubygems there are significantly more downloads of stellar-base than there are of stellar-sdk in total, but for the most recent versions the sdk trumps the base, so it probably suggests that most people are using the sdk with the base rather than the base without the sdk. https://rubygems.org/gems/stellar-sdk So I think moving things into the sdk is good. Simply moving the code from one to the other will cause errors for folks who are already importing them though because they could end up with duplicate or redefined classes in two gems. I think we should consider a couple approaches:
|
I just ran a little experiment with gems and their dependencies. I created a
I created a
This is the current setup we have between the two projects. Then, Note that Then I ran:
And it loaded successfully. This means that removing the dependency from the If I then ran
The class originates from |
So @leighmcculloch I don't think we need to release a new version for We can copy its contents into |
That sounds great. I think this works assuming a developer hasn't included both stellar-sdk and stellar-base in their gemspec or Gemfile. If they have, it won't matter that stellar-sdk has been updated and had the dependency removed from there, there will still be errors. In this case is probably fine though, the error will prompt them to remove the stellar-base dependency. Do we know what that ☝️ error is, and if it will be clear enough what they should do to fix it? We might be able to detect in stellar-sdk that stellar-base has been loaded and raise a very descriptive and clear error. We should be able to do that by checking that any type of value in base exists. @JakeUrban Thoughts? Is that something you could test in your tmpgem project? |
I think it pretty safe to assume they won't have both projects explicitly required, it'll be one or the other. If they do have both, and they We can notify the user by raising an error from |
@JakeUrban, I wouldn't be so sure about that. If someone is using |
Both projects use the same module. So when you Anyone who has both projects |
I don't think we should assume they aren't both explicitly required. Somebody is probably doing this. But we should help get people off base if they're continuing to use an old base with the new sdk that contains base. I think it's important we get people off it because within no time it will be out of date and incompatible with new XDR messages. I'd favor a solution where if you input a new stellar-sdk with an old stellar-base we raise an error. It's preferable if that error is meaningful though and tells them exactly what's going on and what they need to do. |
With astroband/ruby-stellar-sdk#86 we now have stellar-base merged into ruby-stellar-sdk repo in the |
Well, the stellar-base was merged in the master branch of stellar-sdk 🎉 So I close this issue for now. Feel free to open a new one in ruby-stellar-sdk repo, if you have any issues or questions |
This issue is for discussing the pros and cons of keeping the separation of stellar-ruby-sdk and stellar-ruby-base (a.k.a. sdk, base). Currently, everything not related to horizon is kept in base, while the sdk has the horizon hyperclient, transaction paging, and SEP10 helper functions.
Why would we want to keep these separate?
Why would we want to merge them?
Stellar
module, its not always clear where the functionality is defined. For example,Stellar::Account
is not in base, its in the sdk. This isn't a huge deal but its a bit annoying to have 2 projects open and figure out where a particular class is implemented.I'll add more as I think of them.
@overcat I know the Python SDK was originally 2 separate projects, but everything was merged into py-stellar-base. Can you elaborate on what led to that decision?
The text was updated successfully, but these errors were encountered: