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

Generate metadata explicitly for the contract which is build #174

Merged
merged 10 commits into from
Feb 18, 2021
Merged

Generate metadata explicitly for the contract which is build #174

merged 10 commits into from
Feb 18, 2021

Conversation

cmichi
Copy link
Collaborator

@cmichi cmichi commented Feb 12, 2021

Closes #60.

@cmichi cmichi requested a review from ascjones February 12, 2021 16:50
@ascjones
Copy link
Collaborator

Nice! I'll have a proper look on Monday.

ascjones
ascjones reviewed Feb 15, 2021
View reviewed changes
src/workspace/mod.rs Outdated Show resolved Hide resolved
cmichi
cmichi commented Feb 16, 2021
View reviewed changes
src/workspace/mod.rs Outdated Show resolved Hide resolved
ascjones
ascjones requested changes Feb 17, 2021
View reviewed changes
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

This introduces a regression, whereby the optimized contract is empty. Building flipper with

  • master
    image

  • this PR
    image

Haven't tried debugging it, but I think it's worth adding an assert for this in ine of the existing build tests to prevent this regression.

This was referenced Feb 18, 2021
Open
Open
ascjones
ascjones approved these changes Feb 18, 2021
View reviewed changes
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM, this does fix the issue of metadata generation for sub contracts.

However it doesn't completely close all the points in #60, but I have raised a follow up issue for that #182.

Also I came across #181 while testing, which also needs investigating but possibly as a follow up.

@ascjones ascjones mentioned this pull request Feb 18, 2021
Closed
@cmichi cmichi merged commit 594b637 into master Feb 18, 2021
@cmichi cmichi deleted the cmichi-fix-isolated-sub-contract-building branch February 18, 2021 18:45
@cmichi cmichi mentioned this pull request Feb 19, 2021
Merged
@ascjones ascjones mentioned this pull request Feb 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ascjones ascjones ascjones approved these changes

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

Successfully merging this pull request may close these issues.

Generate metadata for sub-contracts in multi contract workspace
2 participants
@cmichi @ascjones