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.

Sign up for GitHub

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

Jump to bottom

Get vault owners list #107

Merged
merged 7 commits into from
Apr 21, 2020
Merged

Get vault owners list #107

merged 7 commits into from
Apr 21, 2020

Conversation

haythemsellami
Copy link
Member

@haythemsellami haythemsellami commented Apr 15, 2020 •
edited
Loading

  • Changed the visibility of vaultOwners array so that it can be fetched through web3js call.

@haythemsellami haythemsellami self-assigned this Apr 15, 2020
@haythemsellami haythemsellami requested a review from aparnakr April 16, 2020 12:00
@haythemsellami haythemsellami changed the title Get list of vaults owners Contracts improvements Apr 16, 2020
@haythemsellami haythemsellami changed the base branch from master to dev April 16, 2020 12:01
This was linked to issues Apr 16, 2020
Add interface for OptionsExchange contract #110
Open
Change list of vault owners visibility #111
Closed
@haythemsellami haythemsellami changed the title Contracts improvements WIP - Contracts improvements Apr 17, 2020
@haythemsellami haythemsellami changed the title WIP - Contracts improvements Get vault owners list Apr 17, 2020
@haythemsellami haythemsellami removed a link to an issue Apr 17, 2020
Add interface for OptionsExchange contract #110
Open
aparnakr
aparnakr reviewed Apr 21, 2020
View reviewed changes
Copy link
Contributor

@aparnakr aparnakr left a comment

Choose a reason for hiding this comment

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

The changes look good! Can you also run the local tests and see if changing this breaks any existing tests? If so maybe modify the tests / have new ones?

@haythemsellami
Copy link
Member Author

The changes look good! Can you also run the local tests and see if changing this breaks any existing tests? If so maybe modify the tests / have new ones?

Existent tests working well on CircleCI... will merge this PR and add tests to this function in another PR.

@haythemsellami haythemsellami merged commit 2c330f8 into dev Apr 21, 2020
@haythemsellami haythemsellami deleted the get-vault-owners-list branch May 19, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aparnakr aparnakr aparnakr left review comments

Assignees

@haythemsellami haythemsellami

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change list of vault owners visibility
2 participants
@haythemsellami @aparnakr