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

feature: moved subgraph from OSx #4

Merged
merged 31 commits into from
Mar 25, 2024
Merged

feature: moved subgraph from OSx #4

merged 31 commits into from
Mar 25, 2024

Conversation

heueristik
Copy link

@heueristik heueristik commented Mar 11, 2024 •
edited
Loading

This PR moves the multisig subgraph files from OSx over.

Task ID: OS-474

Subgraph URL ✅

@heueristik heueristik changed the base branch from main to develop March 11, 2024 17:23
@heueristik heueristik requested review from Rekard0 and jordaniza March 11, 2024 17:27
@heueristik heueristik marked this pull request as ready for review March 11, 2024 17:27
jordaniza
jordaniza reviewed Mar 12, 2024
View reviewed changes
Rekard0
Rekard0 reviewed Mar 12, 2024
View reviewed changes
Copy link

@Rekard0 Rekard0 left a comment

Choose a reason for hiding this comment

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

Over all good work, my general comments would be:

  • Perhaps it need some file restructuring, as i commented
  • Add PR description
  • Create and deploy so we can do a QA

@heueristik heueristik requested a review from Rekard0 March 12, 2024 10:18
heueristik added a commit that referenced this pull request Mar 15, 2024
@heueristik
Merge pull request #4 from aragon/feature/ci-cd
db98da3 
feature: add subgraph workflow
@heueristik
Copy link
Author

Subgraph seems to work: https://subgraphs.alchemy.com/subgraphs/4504/versions/14521
It still needs to be checked via the playground

@heueristik
Copy link
Author

Subgraph seems to work: https://subgraphs.alchemy.com/subgraphs/4504/versions/14521 It still needs to be checked via the playground

LGTM. But: don't trust - verifiy 😄

@heueristik heueristik requested a review from clauBv23 March 15, 2024 16:17
@jordaniza
Copy link

I had a play around with the subgraph you posted. I couldn't find any obvious issues so I think this looks good. You may wish to standardise the naming of the plugins with myself and claudia's PRs

@heueristik
Copy link
Author

I had a play around with the subgraph you posted. I couldn't find any obvious issues so I think this looks good. You may wish to standardise the naming of the plugins with myself and claudia's PRs

What would this mean concretely? Sarkawt suggested that we leave things named as Plugin, also for the file names. The idea is that this reduces merge conflicts and allows git to track things better.

@heueristik heueristik requested a review from jordaniza March 25, 2024 13:24
clauBv23
clauBv23 approved these changes Mar 25, 2024
View reviewed changes
Copy link

@clauBv23 clauBv23 left a comment

Choose a reason for hiding this comment

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

LGTM!

@heueristik heueristik merged commit 1ada3d5 into develop Mar 25, 2024
5 checks passed
@heueristik heueristik deleted the feature/subgraph branch March 25, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clauBv23 clauBv23 clauBv23 approved these changes

@jordaniza jordaniza jordaniza approved these changes

@Rekard0 Rekard0 Awaiting requested review from Rekard0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@heueristik @jordaniza @Rekard0 @clauBv23