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

adding generate params command #6601

Merged

Conversation

polatengin
Copy link
Member

Fixes #512

Contributing a feature

  • I have opened a new issue for the proposal, or commented on an existing one, and ensured that the Bicep maintainers are good with the design of the feature being implemented
  • I have included "Fixes #{issue_number}" in the PR description, so GitHub can link to the issue and close it when the PR is merged

/cc: @majastrz , @ucheNkadiCode

@alex-frankel
Copy link
Collaborator

Thanks for the contribution @polatengin!

Can you clarify the intended usage of the feature? I looked in the command bar in VS code and tried right-clicking on the file name, but didn't see anything in either location. Is this implemented in the CLI?

@ucheNkadiCode
Copy link
Contributor

@shenglol and @anthony-c-martin when either of you have time.

From my understanding, the updates that were made were to make this command available in the Bicep CLI as well as the Command Pallete. Thank you!

Questions for @polatengin,

  1. I also believe this overwrites a parameters file of the same name each time (e.g main.parameters.json) it is called correct Engin? Or does it update the file to append params that are not found and keeps all of the existing params?
  2. Does this also implement telemetry so we can tell how often customers use this feature? Thank you!

@polatengin
Copy link
Member Author

hey @ucheNkadiCode ,

You're right, GenerateParams implemented in both the CLI and the VSCode Extension (I also added required structures into the Language Server)

  1. I also believe this overwrites a parameters file of the same name each time (e.g main.parameters.json) it is called correct Engin? Or does it update the file to append params that are not found and keeps all of the existing params?

Yes, it overwrites the parameters file, if there is already one. if this is not the expected behavior, I can change it.

  1. Does this also implement telemetry so we can tell how often customers use this feature? Thank you!

I didn't implemented telemetry, let me check how can I do it.

polatengin and others added 10 commits June 6, 2022 14:44
…r.cs

Co-authored-by: Stephen Weatherford (MSFT) <StephenWeatherford@users.noreply.github.com>
…r.cs

Co-authored-by: Stephen Weatherford (MSFT) <StephenWeatherford@users.noreply.github.com>
…r.cs

Co-authored-by: Stephen Weatherford (MSFT) <StephenWeatherford@users.noreply.github.com>
Co-authored-by: Stephen Weatherford (MSFT) <StephenWeatherford@users.noreply.github.com>
…r.cs

Co-authored-by: Stephen Weatherford (MSFT) <StephenWeatherford@users.noreply.github.com>
}

var emitter = new TemplateEmitter(compilation.GetEntrypointSemanticModel(), emitterSettings);
using var fileStream = new FileStream(compiledFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite);
Copy link
Member

@anthony-c-martin anthony-c-martin Jun 7, 2022

Choose a reason for hiding this comment

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

I don't think FileMode.OpenOrCreate is what you want here - it means you'll potentially only overwrite part of the file if you write fewer bytes than the size of the existing file.

For example, if I run the command to generate the file, add some newlines to pad it a bit, and run the command again, I get the following (note lines 28-30 left behind from the old file after the JSON ends):
image

…nature of emitparametersfile methods, updating tofile method signature accordingly
Copy link
Member

@anthony-c-martin anthony-c-martin left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for contributing!

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.

Auto-generate parameter files
8 participants