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

reserve comments for templates #913

Merged
merged 3 commits into from
Mar 23, 2021
Merged

Conversation

timqian
Copy link
Contributor

@timqian timqian commented Mar 23, 2021

issue: https://app.asana.com/0/1200011502754281/1200078207478206

This PR helps to reserve the comments in the serverless.yml of templates

Before

sls init eggjs-starter
Screen Shot 2021-03-23 at 1 25 58 PM

After

Screen Shot 2021-03-23 at 1 26 42 PM

@timqian
Copy link
Contributor Author

timqian commented Mar 23, 2021

The reason we lost the comments of serverless.yml after sls init a template is because we need to change the app key in the yml. Currently, we firstly turn this yml to an object, update this object and then recreate a yml. In this process, the comments are lost.

Unfortunately, js-yaml is not able to persist comments while parsing yml: nodeca/js-yaml#549

There are two ways to fix this issue.

Option 1:

Use libs that preserve comments.

Disadvatages

Option 2:

Use RegExp to replace app(current implementation)

I have tested this on the following 4 situations, it works well

  • Template yml with comments
  • Template yml without comments
  • Rename app when sls init, e.g. sls init eggjs-starter --name hello
  • Use default app name when sls init

@timqian timqian requested review from ole3021 and zongUMR March 23, 2021 05:46
@timqian timqian merged commit 359c211 into master Mar 23, 2021
@timqian timqian deleted the reserve-comments-for-templates-yml branch March 23, 2021 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants