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.

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

chore: document subgraph schema #583

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

banasa44
Copy link
Contributor

@banasa44 banasa44 commented Apr 29, 2024

Description

2nd part of the inline comments for subgraph schema. These are meant to be reflected in our auto-generated documentation (subgraph/reference-guide) in our developer-portal.

https://subgraph.satsuma-prod.com/aragon/osx-carles-sepolia/playground

Task ID: OS-1206

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran all tests with success and extended them if necessary.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have updated the DEPLOYMENT_CHECKLIST file in the root folder.
  • I have updated the UPDATE_CHECKLIST file in the root folder.
  • I have updated the Subgraph and added a QA URL to the description of this PR.

Comment on lines +492 to +494
" Tokens held by the DAO. "
token: ERC20Contract

Copy link
Contributor

Choose a reason for hiding this comment

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

@Michael-A-Heuer for visibility
this attribute is not accurate any more, but also need some planning

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not?

balances: [TokenBalance!]! @derivedFrom(field: "dao")

" The permissions given to a DAO to govern its actions (refer to `Objects/Permission` in this documentation site for more information). "
Copy link
Contributor

@Rekard0 Rekard0 Apr 29, 2024

Choose a reason for hiding this comment

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

only commenting it here
thinks like : (refer to Objects/Permission in this documentation site for more information) does this create an actual link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it doesn't, I haven't add a proper link as it will feel super odd when seeing the schema (as it would be a relative path from another repo). I can add it though

Comment on lines 500 to 505
" The address of the trusted forwarder required for meta transactions, if added while creating the DAO. "
trustedForwarder: Bytes

" Deprecated field. "
signatureValidator: Bytes

Copy link
Contributor

@Rekard0 Rekard0 Apr 29, 2024

Choose a reason for hiding this comment

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

@Michael-A-Heuer what should we do with trustedForwarder ? is't this also to be deprecated

@banasa44 banasa44 requested review from heueristik and removed request for clauBv23 and jordaniza April 29, 2024 11:28
packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
Comment on lines +492 to +494
" Tokens held by the DAO. "
token: ERC20Contract

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not?

packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
packages/subgraph/schema.graphql Show resolved Hide resolved
packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
packages/subgraph/schema.graphql Show resolved Hide resolved
packages/subgraph/schema.graphql Outdated Show resolved Hide resolved
helpers: [Bytes!]!

" The array of multi-targeted permission operations to be applied by the `PluginSetupProcessor` to install/uninstall/update the plugin in the DAO. "
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you strictly need to apply multi target permissions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are inside PluginPreparation entity, which has permissions that will be handled by the psp which only uses applyMultiTargetPermission function

banasa44 and others added 2 commits April 30, 2024 10:58
Co-authored-by: Rekard0 <5880388+Rekard0@users.noreply.github.com>
Co-authored-by: Jordan <45881807+jordaniza@users.noreply.github.com>
@banasa44 banasa44 changed the title Chore/document subgraph schema chore: document subgraph schema Apr 30, 2024
@jordaniza jordaniza marked this pull request as draft May 3, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants