-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
feat: Adding packagemeta to config and project manager #802
Changes from 13 commits
d6ef1df
06de772
67ccc3a
93be732
cd76db4
2c9526a
9e6539c
099d610
3e47528
e892b36
ed41979
0b68bf1
218fe1c
6a06e23
d8537b2
230b1b5
863083d
f97e1d2
c72aba2
552ce43
b6b9f74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,11 @@ | |
from pathlib import Path | ||
from typing import Dict, List, Optional, Type, Union | ||
|
||
from ethpm_types import Compiler, ContractType, PackageManifest | ||
from ethpm_types import Compiler, ContractType, PackageManifest, PackageMeta | ||
|
||
from ape.api import DependencyAPI, ProjectAPI | ||
from ape.contracts import ContractContainer, ContractNamespace | ||
from ape.exceptions import ProjectError | ||
from ape.exceptions import ConfigError, ProjectError | ||
from ape.managers.base import BaseManager | ||
from ape.managers.project.types import ApeProject, BrownieProject | ||
|
||
|
@@ -447,16 +447,23 @@ def _get_contract(self, name: str) -> Optional[ContractContainer]: | |
|
||
return None | ||
|
||
# @property | ||
# def meta(self) -> PackageMeta: | ||
# return PackageMeta(**self.config_manager.get_config("ethpm").serialize()) | ||
@property | ||
def meta(self) -> PackageMeta: # type: ignore | ||
""" | ||
Populate package manifest with metadata as per EIP | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not populate the manifest, so I would change this doc. Try something like
|
||
https://eips.ethereum.org/EIPS/eip-2678#the-package-meta-object | ||
""" | ||
try: | ||
return self.config_manager.meta | ||
except Exception as e: | ||
raise ConfigError(f"Incorrect configuration of package metadata:\n{meta}") from e | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @unparalleled-js I'm still having this issue and not sure how to resolve it. |
||
|
||
# def publish_manifest(self): | ||
# manifest = self.manifest.dict() | ||
# if not manifest["name"]: | ||
# raise ProjectError("Need name to release manifest") | ||
# if not manifest["version"]: | ||
# raise ProjectError("Need version to release manifest") | ||
# TODO: Clean up manifest and minify it | ||
|
||
# TODO: Publish sources to IPFS and replace with CIDs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this would require looping through all the sources and publishing them to IPFS as well, then updating all of the links in the package from local links to global ones. Also side-note is that eventually |
||
# TODO: Publish to IPFS | ||
Comment on lines
466
to
467
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs #208 to complete these TODOs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this required for this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we are building towards that though with the Deployments story There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #365 also this related issue for IPFS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably put the type ignore on the return stmt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this fixed the type issue.