search

OpenST / openst-contracts

OpenST is a framework for building token economies on Ethereum
Apache License 2.0
26 stars 19 forks source link

Use git-submodules to include SafeMath and Organized contracts in openst-contracts #197

Closed deepesh-kn closed 5 years ago

deepesh-kn commented 5 years ago

Use git-submodules to include the following contracts

Following is the proposal:

  1. Remove the existing implementation of the above contracts.
  2. Use git-submodules to include Organized contract in the project from OpenST/organization-contracts.
  3. Use git-submodules to include SafeMath contract in the project from < will be updated >
0xsarvesh commented 5 years ago

@deepesh-kn SafeMath.sol is owned by openzeppelin . See here

They also have published npm. See here

Questions:

  1. is openzeppelin correct source for safemath for this ticket?
  2. Why do you prefer git-submodule instead of npm for this?
deepesh-kn commented 5 years ago

@deepesh-kn SafeMath.sol is owned for openzeppelin . See here

They also have published npm. See here

Questions:

  1. is openzeppelin correct source for safemath for this ticket?

Yes, openzeppelin is the correct source for safemath.

  1. Why do you prefer git-submodule instead of npm for this?

I am open for the discussion here. We have already used git-submodules in other repo so it was suggested here. We can use NPM for this. @pgev, you have done the research on git-submodule vs NPM and used git-submodules in one of the repo. What is your opinion here to use NPM?

0xsarvesh commented 5 years ago

I would choose the npm package instead of git-submodule for dependency in this case.

Reasoning: We can selectively import contracts like safemath.sol in npm package on the flip side all the contracts files will be pulled into the repository with git-submodule. With this reasoning, the npm package approach seems cleaner.

Thoughts? @deepesh-kn @schemar @pgev

schemar commented 5 years ago

I would choose the npm package instead of git-submodule for dependency in this case.

Reasoning: We can selectively import contracts like safemath.sol in npm package on the flip side all the contracts files will be pulled into the repository with git-submodule. With this reasoning, the npm package approach seems cleaner.

Thoughts? @deepesh-kn @schemar @pgev

Please note that only a reference to the other repo is stored in our repo (upstream) in case of git submodules. It gets only expanded locally on the dev machines.

However, if we cannot find a way to pin specific tags with git submodules, they are not an option in my opinion. We need to be able to select versions of our dependencies. @sarvesh-ost can you find out if you can specify a specific tag in the .gitsubmodules file so that everyone would clone that specific tag? Selecting just a branch (e.g. master) I would deem insufficient.