Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Add noaddon option #8

Closed
wants to merge 2 commits into from
Closed

Add noaddon option #8

wants to merge 2 commits into from

Conversation

sneridagh
Copy link
Member

This is for the use case of the upcoming image. We need no addon in there to have a "clean" Volto if the image alone is run for demo purposes.

@sneridagh sneridagh requested a review from ericof April 3, 2024 07:45
Copy link
Member

@ericof ericof left a comment

Choose a reason for hiding this comment

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

Left some comments about style, but I do have questions about having the whole noaddon option here. Let's talk a bit more

@@ -6,6 +6,7 @@
"email": "collective@plone.org",
"github_organization": "collective",
"npm_package_name": "{{ cookiecutter.addon_name }}",
"noaddon": false,
Copy link
Member

Choose a reason for hiding this comment

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

Please, also add the user friendly version under __prompts__

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -38,8 +44,18 @@ def run_cmd(command: str, shell: bool, cwd: str) -> bool:
return False if proc.returncode else True


def remove(filepath):
if os.path.isfile(filepath):
Copy link
Member

Choose a reason for hiding this comment

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

Use pathlib.Path

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -34,8 +34,12 @@
},
"dependencies": {
"@plone/volto": "workspace:*",
{% if not cookiecutter.noaddon -%}
Copy link
Member

Choose a reason for hiding this comment

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

Why not to use:

  "dependencies": {
    "@plone/volto": "workspace:*",
    "@plone/registry": "workspace:*"{%- if not cookiecutter.noaddon -%},
    "{{ cookiecutter.npm_package_name }}": "workspace:*"
    {%- endif %}
  },

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -1,4 +1,8 @@
{% if not cookiecutter.noaddon -%}
Copy link
Member

Choose a reason for hiding this comment

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

Why not to use:

const addons = [{%- if not cookiecutter.noaddon -%}'{{ cookiecutter.npm_package_name }}'{%- endif -%}];

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@sneridagh
Copy link
Member Author

@ericof It's needed for the generation of the image without add-on. It can be even be a hidden feature. If not, when we generate the image, there will be always the default add-on in there.

@ericof
Copy link
Member

ericof commented Apr 5, 2024 via email

@sneridagh
Copy link
Member Author

Let's do this in the image stage, not having a public facing way to do this.

@sneridagh sneridagh closed this Apr 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants