replace golang with js

[geofmureithi]

Referenced issue

96

Summary of changes

We are already using js in our tests, This PR attempts to keep with only this dependency

Reviewer recommendations

The PR does not work completely. If more work is needed please advice appropriately.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eiger-nebula	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 10, 2023 10:52pm

[geofmureithi]

Looks like CI is not passing yet. Thus the only reason because i do not approve this yet.

It shouldn't pass though. The code I added is the sample code from stellar. I was just converting the scripts.

From my perspective, things like CLIs/tooling are better in compiled languages, as no need for dep download and project config.

Yes, but it is not that simple. While the approach you used worked, it has one design flow, tests only pass in the linux architecture you built it for (x86-64). This wont work in arm, windows or mac (eg those tests are broken in my local mac build). For anyone to get it to run in other environments, one would have to install go, know how to compile, and then run tests or you would have to provide an executable for all plausible architectures. That is the real deal breaker.

But feel free to change it as you want and to make it work :) I was doing this as a POC for the language not the inner code, you worked on it initially, you might be better (why its pulling to your branch not main). I can try work it out if its going to be a hassle for you.

All in all, I wish we had discussed this decision (to add golang) before implementation because there are choices that were applicable for you. Here is cargo-make's list of supported programming languages.

I hope this clears it up, tell me if you are ok implementing the asset creation part or I can try to port your code.

[eloylp]

Looks like CI is not passing yet. Thus the only reason because i do not approve this yet.

It shouldn't pass though. The code I added is the sample code from stellar. I was just converting the scripts.

From my perspective, things like CLIs/tooling are better in compiled languages, as no need for dep download and project config.

Yes, but it is not that simple. While the approach you used worked, it has one design flow, tests only pass in the linux architecture you built it for (x86-64). This wont work in arm, windows or mac (eg those tests are broken in my local mac build). For anyone to get it to run in other environments, one would have to install go, know how to compile, and then run tests or you would have to provide an executable for all plausible architectures. That is the real deal breaker.

But feel free to change it as you want and to make it work :) I was doing this as a POC for the language not the inner code, you worked on it initially, you might be better (why its pulling to your branch not main). I can try work it out if its going to be a hassle for you.

All in all, I wish we had discussed this decision (to add golang) before implementation because there are choices that were applicable for you. Here is cargo-make's list of supported programming languages.

I hope this clears it up, tell me if you are ok implementing the asset creation part or I can try to port your code.

Lets take into account this script its not a plain JS script which only uses very stable APIs of the standard library. It has dependencies on the Stellar SDK, which probably has requirements in terms of the local installed NodeJS version , which may differ machine to machine.

Although this script is mostly intended to be executed in the CI, i do agree we might add the script to the .gitignore file, so each machine just need todo a go build one time. If its documented, i do not see an issue here.

Thus from my perspective, this is still a matter of tradeoffs and probably personal preferences. My opinion is to keep the golang one, but is not a strong one. I am open if you wish to complete the script in JS and see how it goes :)

[geofmureithi]

which probably has requirements in terms of the local installed NodeJS version , which may differ machine to machine.

I think that's the case for every js developer working on StellarSDK. We don't all have the same version.

We already require node.js for our scripts, lets keep it simple. I am finishing on the js part. Should be ready today. We should extract your code to an nft builder as you suggested, and it can even have more entries eg NFT name, amount etc.

[geofmureithi]

@eloylp I am not sure what you mean by this is a cli. I was assuming this specific to tests. I mean just like all other js scripts. It could be used as cli but thats out of the scope of tests. I was expecting your golang code will be converted to a cli, in a separate repo.

[eloylp]

@eloylp I am not sure what you mean by this is a cli. I was assuming this specific to tests. I mean just like all other js scripts. It could be used as cli but thats out of the scope of tests. I was expecting your golang code will be converted to a cli, in a separate repo.

Well, it accepts input parameters and could be used in another parts. But yeah we have the code in the history, we can recover it anytime if want to use it later.

[geofmureithi]

I am encouraging you to do it right now.(If it doesn't take much of your time) It can be a small quick win. It shouldn't take much as you already had the Readme and the code. The only extra param you will need is the nft code.

[eloylp]

I am encouraging you to do it right now.(If it doesn't take much of your time) It can be a small quick win. It shouldn't take much as you already had the Readme and the code. The only extra param you will need is the nft code.

Actually, i think its not the time for priorizing that, as that will require to think how to make it more useful, with more input parameters, like hashes or other metadata of the NFT. But, we could create an issue to not forget about it and find a time slot in the future.