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

Add mustache to dev env #5819

Merged
merged 11 commits into from
Aug 7, 2023
Merged

Conversation

smaarn
Copy link
Contributor

@smaarn smaarn commented Jul 22, 2023

Description

mustache is a very simple template language which could be used to easily generate internationalized wizards.

This is an attempt to integrate it in the dev environment and would be a prerequisite for #5817

Homepage: https://mustache.github.io/

Checklist

Type of change

  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@smaarn smaarn requested a review from th0ma7 July 22, 2023 16:50
@smaarn smaarn marked this pull request as draft July 22, 2023 16:51
@smaarn smaarn marked this pull request as ready for review July 22, 2023 17:57
@mreid-tt mreid-tt removed their request for review July 22, 2023 19:14
@smaarn smaarn mentioned this pull request Aug 6, 2023
6 tasks
@smaarn
Copy link
Contributor Author

smaarn commented Aug 6, 2023

TBH there is this still the documentation which could be added about the "how to use it".

- avoid too long RUN command (max length is 1024 chars)
- add zip package to dev env.
@hgy59
Copy link
Contributor

hgy59 commented Aug 6, 2023

@smaarn I had to break the identation to limit the single RUN command to install packages.

and added zip package for a future removement of native/zip...

@th0ma7, @publicarray, @mreid-tt can you have a second look on the wizard generation from templates?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done.

Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

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

LGTM

@hgy59
Copy link
Contributor

hgy59 commented Aug 7, 2023

@smaarn
BTW normally I used to add a custom build step to install packages before they are available in the dev env with

   sudo apt-get update
   sudo apt-get install ruby-mustache

but due to the sudo file error (fixed here) this did not work in spksrc containers on my newer dev machine with debian bookworm.

@smaarn
Copy link
Contributor Author

smaarn commented Aug 7, 2023

I reworked to add "native" variables usable in switches:

  • IS_DSM_6_OR_GREATER (true if DSM version is >= 6.0)
  • IS_DSM_7_OR_GREATER (true if DSM version is >= 7.0)
  • IS_DSM_6 (true if DSM version >= 6.0 and < 7.0)
  • IS_DSM_7 (true if DSM version >= 7.0)

This allows to tailor templates depending on the DSM version (without even using shell scripts !)

@smaarn smaarn merged commit 63de214 into SynoCommunity:master Aug 7, 2023
2 checks passed
@smaarn smaarn deleted the chore/integrate-mustache branch August 7, 2023 16:18
@hgy59 hgy59 mentioned this pull request Aug 7, 2023
2 tasks
@mreid-tt
Copy link
Contributor

mreid-tt commented Oct 15, 2023

@smaarn, I see the wizards in cops and I like them. I'd like to incorporate in the next update to owncloud. Will the template also work for install_uifile.sh files? If so, would the language file be called install_uifile.sh.yml?

I also note you introduced a new variable WIZARDS_TEMPLATES_DIR but I don't see documentation of how to use it in https://github.com/SynoCommunity/spksrc/wiki/Makefile-variables. Perhaps an update there and a mention on it's use with WIZARDS_DIR?

EDIT: I've tried a build for owncloud using mustache for wizards however no files were generated as part of #5912. Can you help review where I may have gone wrong?

@mreid-tt mreid-tt mentioned this pull request Oct 15, 2023
8 tasks
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