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

Create registry #9

Merged
merged 19 commits into from
Jun 23, 2023
Merged

Create registry #9

merged 19 commits into from
Jun 23, 2023

Conversation

akshay-ap
Copy link
Contributor

@akshay-ap akshay-ap commented Jun 16, 2023 •
edited
Loading

Fixes #3

Merge only after merging #8

Changes in PR:

  • Test SafeProtocolRegistry
  • Create contract SafeProtocolRegistry
  • Add new commands for coverage, ts file formatting

SafeProtocolRegistry:

  • Is Ownable
  • Only Owner can add component
  • Only owner can flag component
  • Once added, component cannot be removed
  • Implements ISafeProtocolRegistry interface (check(...) method)
  • Has natspec docstrings

@akshay-ap akshay-ap self-assigned this Jun 16, 2023
@akshay-ap akshay-ap marked this pull request as draft June 16, 2023 14:41
@akshay-ap akshay-ap changed the title Create registry WIP: Create registry Jun 16, 2023
@akshay-ap akshay-ap force-pushed the feature-#3-create-registry branch from d019c16 to d4c22a8 Compare June 18, 2023 16:02
@rmeissner
Copy link
Member

I would recommend you to set the base of the PR to the branch that you based it on (i.e. the one with the interface). It is possible to change this later and reduces the noise in changes.

@akshay-ap akshay-ap changed the title WIP: Create registry Create registry Jun 19, 2023
@akshay-ap akshay-ap marked this pull request as ready for review June 19, 2023 18:07
@akshay-ap akshay-ap requested review from mmv08 and rmeissner June 19, 2023 18:07
mmv08
mmv08 reviewed Jun 21, 2023
View reviewed changes
Copy link
Member

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

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

Have you seen error messages from the linter?

@akshay-ap
Copy link
Contributor Author

akshay-ap commented Jun 23, 2023 •
edited
Loading

Have you seen error messages from the linter?

Yup. all are errors are fixed and in #12 all warnings as well

@akshay-ap akshay-ap merged commit f85de6f into main Jun 23, 2023
@akshay-ap akshay-ap deleted the feature-#3-create-registry branch June 23, 2023 16:14
akshay-ap added a commit that referenced this pull request Sep 8, 2023
@akshay-ap
[#9] Update natspec doc
9992a18
akshay-ap added a commit that referenced this pull request Sep 8, 2023
@akshay-ap
[#9] Update comment
3c9c732
github-merge-queue bot pushed a commit that referenced this pull request Sep 11, 2023
@akshay-ap
Implement fine grained permissions (#98)
3a6a6a3 
* [#97] Implement fine grained permissions

* [#97] Use constants for permissions

* [#97] Create a function for checking permissions

* [#97] Update README.md and natspec doc

* [#9] Update comment

* [#97] Remove redundant storage update statement, remove unused import in TestPlugin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmv08 mmv08 mmv08 left review comments

@rmeissner rmeissner rmeissner approved these changes

Assignees

@akshay-ap akshay-ap

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement simple registry
3 participants
@akshay-ap @rmeissner @mmv08