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

Added generated metadata info #1533

Merged
merged 21 commits into from
Feb 18, 2021
Merged

Added generated metadata info #1533

merged 21 commits into from
Feb 18, 2021

Conversation

TSunny007
Copy link
Contributor

@TSunny007 TSunny007 commented Feb 11, 2021

Contributing a Pull Request

Fixes #800

Contributing to documentation

  • The contribution does not exist in any of the docs in either the root of the docs directory or the specs

Contributing an example

  • I have checked that there is not an equivalent example already submitted
  • I have resolved all warnings and errors shown by the Bicep VS Code extension
  • I have checked that all tests are passing by running dotnet test
  • I have consistent casing for all of my identifiers and am using camelCasing unless I have a justification to use another casing style

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
  • I have appropriate test coverage of my new feature

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.

See comments

@codecov-io
Copy link

codecov-io commented Feb 14, 2021

Codecov Report

Merging #1533 (4d5618c) into main (eba2be2) will increase coverage by 0.01%.
The diff coverage is 98.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1533      +/-   ##
==========================================
+ Coverage   94.86%   94.87%   +0.01%     
==========================================
  Files         356      356              
  Lines       19034    19082      +48     
  Branches       14       14              
==========================================
+ Hits        18056    18104      +48     
  Misses        978      978              
Flag Coverage Δ
dotnet 95.36% <98.76%> (+0.01%) ⬆️
typescript 27.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Bicep.Core/LanguageConstants.cs 99.04% <ø> (ø)
src/Bicep.Cli/Program.cs 93.68% <80.00%> (+0.06%) ⬆️
src/Bicep.Cli.IntegrationTests/DecompileTests.cs 100.00% <100.00%> (ø)
src/Bicep.Cli.IntegrationTests/ProgramTests.cs 100.00% <100.00%> (ø)
...Core.IntegrationTests/Emit/TemplateEmitterTests.cs 100.00% <100.00%> (ø)
src/Bicep.Core.IntegrationTests/ModuleTests.cs 97.43% <100.00%> (ø)
src/Bicep.Core.Samples/ExamplesTests.cs 99.00% <100.00%> (ø)
...rc/Bicep.Core.UnitTests/Utils/CompilationHelper.cs 100.00% <100.00%> (ø)
src/Bicep.Core/Emit/TemplateEmitter.cs 100.00% <100.00%> (ø)
src/Bicep.Core/Emit/TemplateWriter.cs 98.19% <100.00%> (+0.11%) ⬆️
... and 5 more

/// <summary>
/// Since emitting modules is a nested call to this, it behaves differently (we reuse the main memoryWriter)
/// </summary>
public void Write(bool moduleEmit)
Copy link
Member

Choose a reason for hiding this comment

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

What are your thoughts on:

  1. Moving the logic for moduleEmit == false directly into the Write() overload without params
  2. Making this method private, renaming it to WriteModule(), and having it just handle the module case (remove the param).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack! It didn't occur to me that that i could simply rename

moduleWriter.Write();

to WriteModule. Nice suggestion!

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.

LGTM - just one minor comment

@TSunny007 TSunny007 merged commit e0e0c4c into main Feb 18, 2021
@TSunny007 TSunny007 deleted the tsunkaraneni/generatedMetadata branch February 18, 2021 18: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.

Bicep-generated files should include an autogenerated header
4 participants