Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove duplicate package.json, update readme. #33

Closed
wants to merge 4 commits into from
Closed

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Nov 6, 2018

  • Remove the duplicate package.json and it's reference.
  • Add root README to manifest.
  • Replace duplicate package.json references with root.
  • Update pytest usage info in generated readme.

Closes #30, Close #31

@@ -17,7 +17,7 @@
_sys.exit(1)

_basepath = _os.path.dirname(__file__)
_filepath = _os.path.abspath(_os.path.join(_basepath, 'package.json'))
_filepath = _os.path.abspath(_os.path.join(_basepath, '..','package.json'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm will this actually work once the package is installed via pip?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is why that copyfile was there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a test that will package the build and run an app with it installed as a tarball.

from setuptools import setup


with open(os.path.join('{{cookiecutter.project_shortname}}', 'package.json')) as f:
with open('package.json') as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -9,7 +9,7 @@
"prepublish": "npm run validate-init",
"build:js-dev": "webpack --mode development",
"build:js": "webpack --mode production",
"build:py": "node ./extract-meta.js src/lib/components > {{cookiecutter.project_shortname}}/metadata.json && copyfiles package.json {{cookiecutter.project_shortname}} && %(python_path) -c \"import dash; dash.development.component_loader.generate_classes('{{cookiecutter.project_shortname}}', '{{cookiecutter.project_shortname}}/metadata.json')\"",
"build:py": "node ./extract-meta.js src/lib/components > {{cookiecutter.project_shortname}}/metadata.json && %(python_path) -c \"import dash; dash.development.component_loader.generate_classes('{{cookiecutter.project_shortname}}', '{{cookiecutter.project_shortname}}/metadata.json')\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

see my comment in plotly/dash#451

@nicolaskruchten
Copy link
Contributor

In principle this is mergable once we clear up the need for package.json being copied. Perhaps we should update this PR once plotly/dash#451 is merged and have this PR encompass the new dash-generate-components call pattern?

@nicolaskruchten
Copy link
Contributor

@T4rk1n can we address point 3 of #38 (renaming the copied package.json) in this PR ?

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Feb 6, 2019

Closing as outdated.

@T4rk1n T4rk1n closed this Feb 6, 2019
@T4rk1n T4rk1n deleted the readme-update branch February 6, 2019 15:59
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants