Skip to content

Migrate data of GTM - #538

Merged
satyamakgec merged 15 commits into
dev-3.0.0from
migrate-gtm-data
Feb 6, 2019
Merged

Migrate data of GTM#538
satyamakgec merged 15 commits into
dev-3.0.0from
migrate-gtm-data

Conversation

@satyamakgec

@satyamakgecsatyamakgec commented Feb 1, 2019

Copy link
Copy Markdown
Contributor

No description provided.

@satyamakgecsatyamakgec changed the title migrate data of gtm[WIP]: Migrate data of GTMFeb 1, 2019
@satyamakgecsatyamakgec changed the title [WIP]: Migrate data of GTMMigrate data of GTMFeb 5, 2019
maxsam4
maxsam4 suggested changes Feb 5, 2019
View reviewed changes

@maxsam4maxsam4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only partial review completed yet.

Comment threadcontracts/libraries/VersionUtils.sol Outdated
Comment threadcontracts/libraries/VersionUtils.sol Outdated
@coveralls

coveralls commented Feb 6, 2019•
edited
Loading

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 96.86% when pulling eff3390 on migrate-gtm-data into 25e8f0e on dev-3.0.0.

maxsam4
maxsam4 reviewed Feb 6, 2019
View reviewed changes

@maxsam4maxsam4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left some minor comments regarding optimization.

Comment threadcontracts/modules/TransferManager/GeneralTransferManager.sol Outdated
Comment threadcontracts/modules/TransferManager/GeneralTransferManager.sol
@adamdossa

adamdossa commented Feb 6, 2019

Copy link
Copy Markdown
Contributor

I modified a couple of the function names and added a _validLockTime function to make code a bit more readable.

@adamdossa

adamdossa commented Feb 6, 2019•
edited
Loading

Copy link
Copy Markdown
Contributor

I also wonder if we could do something more sensible with the investors data. We currently have a list of investors (append-only) in the token, and the list of investors (KYC data individuals) in the GTM. We could think about consolidating this in the dataStore possibly in a single data set. This could be a separate PR though.

@satyamakgec
satyamakgec merged commit 22c48b3 into dev-3.0.0Feb 6, 2019
@satyamakgec
satyamakgec deleted the migrate-gtm-data branch October 30, 2019 12:16
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamdossaadamdossaAwaiting requested review from adamdossa
1 more reviewer
@maxsam4maxsam4maxsam4 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@satyamakgec@coveralls@adamdossa@maxsam4@SatyamSB