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

chore: Remove local file support for manifest and defaultCR #81

Conversation

c-pius
Copy link
Contributor

@c-pius c-pius commented Oct 22, 2024

Description

Changes proposed in this pull request:

Related issue(s)

@c-pius c-pius force-pushed the chore/remove-local-file-support-for-manifest-defaultCR branch from f43129b to 4d402f9 Compare October 22, 2024 14:32
@c-pius
Copy link
Contributor Author

c-pius commented Oct 22, 2024

PR on hold. We need to re-discuss the idea of mandating file links. This makes sense for module releases that are put into module-manifests repo. The teams first create the module release, than update the config in the repo...

However, is it necessary for module teams to have the local file option during development of the module? We for instance also use the local file option when testing locally or in the E2E test pipeline. Module teams may do the same to quickly push a working state into a local registry. We need to clarify before we merge this.

@ruanxin
Copy link
Contributor

ruanxin commented Oct 23, 2024

basically, module team should not care about ModuleTemplate, modulectl is a tool design and used by submission process only, there is no needs for module team to create ModuleTemplate for local development, to verify the functionally of module just need to deploy raw manifest yaml directly, like what we offered in quick install guide. Additionally, introducing the option for a local file reference may cause confusion, as it requires the team to have separate configurations for local testing and the submission pipeline, supporting such usage would also increase our effort in terms of documentation and clarification.

Anyway, we can remove this option to simplify the usage first, if later do have the needs to reference local file, we can also support accordingly.

@c-pius c-pius marked this pull request as ready for review October 23, 2024 08:00
@c-pius c-pius changed the title chore: Remove local file support for 'manifest' and 'defaultCR' chore: Remove local file support for manifest and defaultCR Oct 23, 2024
@kyma-bot kyma-bot added the lgtm label Oct 23, 2024
@c-pius c-pius merged commit ab02b3f into kyma-project:main Oct 23, 2024
11 checks passed
@c-pius c-pius deleted the chore/remove-local-file-support-for-manifest-defaultCR branch October 23, 2024 11:03
@c-pius c-pius assigned c-pius and unassigned nesmabadr Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants