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

[Proxy colonies] Feat: Add create event details to colony model #303

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

mmioana
Copy link
Contributor

@mmioana mmioana commented Dec 12, 2024

This PR addresses adding the colony create event details to the Colony model upon colony creation.
This information is needed for easily deploying new Proxy Colonies.

CDapp PR
Scripts PR

Feat: Add create event details to colony model
@mmioana mmioana force-pushed the feat/colony-added-event branch from 754ae12 to 9f9904a Compare December 13, 2024 17:06
@mmioana mmioana changed the title Feat: Add create event details to colony model [Proxy colonies] Feat: Add create event details to colony model Dec 16, 2024
@bassgeta bassgeta marked this pull request as ready for review December 17, 2024 10:25
Copy link
Contributor

@davecreaser davecreaser left a comment

Choose a reason for hiding this comment

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

Nice and simple, good stuff :)

Just one comment / question about error handling. Not 100% sure about the best way to go about it tbh, might need some discussion.

.getTransaction(transactionHash);
signerAddress = transaction.from;
} catch (error) {
// Most likely there was an error retrieving this transaction
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to bail out of the whole handler if this fails? Or at least log an error or something? I guess it's not critical to the colony creation process (only proxy colonies later down the line) so maybe bailing out is too severe? 🤷

@bassgeta bassgeta force-pushed the feat/colony-added-event branch from 93f5d6c to cea3572 Compare December 23, 2024 12:52
Base automatically changed from feat/wormhole-tx-matching to feat/proxy-colonies-dev-env January 13, 2025 10:09
Copy link
Contributor

@bassgeta bassgeta left a comment

Choose a reason for hiding this comment

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

Approving since we worked on it together 💪

Copy link
Contributor

@Nortsova Nortsova left a comment

Choose a reason for hiding this comment

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

Great work! Tested in scope of JoinColony/colonyCDapp#3920 🙌

@bassgeta bassgeta merged commit bcab4c7 into feat/proxy-colonies-dev-env Jan 16, 2025
1 check passed
@bassgeta bassgeta deleted the feat/colony-added-event branch January 16, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants