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

Don't add empty template properties - Issue 221 #713

Merged
merged 7 commits into from
Oct 26, 2020
Merged

Don't add empty template properties - Issue 221 #713

merged 7 commits into from
Oct 26, 2020

Conversation

glav
Copy link
Contributor

@glav glav commented Oct 23, 2020

Fixes #221

Caveats:

  • functions is still output as it hasn't really been implemented yet
  • sometimes 'variables' is still output if there is a 'var x = ....' in there.

@ghost
Copy link

ghost commented Oct 23, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

Added some comments. Please take a look.

@codecov-io
Copy link

codecov-io commented Oct 24, 2020

Codecov Report

Merging #713 into main will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #713      +/-   ##
==========================================
+ Coverage   94.38%   94.43%   +0.05%     
==========================================
  Files         268      272       +4     
  Lines       11150    11451     +301     
  Branches       12       12              
==========================================
+ Hits        10524    10814     +290     
- Misses        626      637      +11     
Flag Coverage Δ
#dotnet 95.20% <100.00%> (+0.03%) ⬆️
#typescript 25.98% <ø> (ø)

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

Impacted Files Coverage Δ
src/Bicep.Core/Emit/EmitterContext.cs 100.00% <100.00%> (ø)
src/Bicep.Core/Emit/InlineDependencyVisitor.cs 93.54% <100.00%> (ø)
src/Bicep.Core/Emit/TemplateWriter.cs 97.40% <100.00%> (+0.10%) ⬆️
src/Bicep.Core/SemanticModel/SemanticModel.cs 95.83% <0.00%> (-4.17%) ⬇️
src/Bicep.Core/Syntax/SyntaxTreeGroupingBuilder.cs 97.67% <0.00%> (-2.33%) ⬇️
....IntegrationTests/Helpers/IntegrationTestHelper.cs 96.42% <0.00%> (-0.87%) ⬇️
src/Bicep.Core/TypeSystem/TypeAssignmentVisitor.cs 98.94% <0.00%> (-0.13%) ⬇️
src/Bicep.Cli/Program.cs 92.50% <0.00%> (ø)
src/Bicep.LangServer/Program.cs 0.00% <0.00%> (ø)
src/Bicep.Core.Samples/ExamplesTests.cs 98.18% <0.00%> (ø)
... and 17 more

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

This is great. Thanks for contributing!

@majastrz
Copy link
Member

The vsix failure is a known issue. I'll retry your build...

@majastrz
Copy link
Member

The build failure might not be transient. Can you check if the E2E tests are passing for you.

If you open the \src\vscode-bicep path in VS code, you should be able to run them with the following task:
image

If that's not working or if you can't figure it out, let me know and I can take a look.

@glav
Copy link
Contributor Author

glav commented Oct 26, 2020

Hmm. I did get some failures. See below. Not really sure how to resolve these at present but will have a look after work today.

× should reveal type signature when hovering over a parameter name (4 ms)

runner.ts:21
× should reveal type signature when hovering over a variable name (19 ms)
runner.ts:21
× should reveal type signature when hovering over a resource symbolic name (2 ms)
runner.ts:21
× should reveal type signature when hovering over an output name (3 ms)
runner.ts:21
× should reveal type signature when hovering over a function name (5 ms)

@majastrz majastrz merged commit f3678a4 into Azure:main Oct 26, 2020
@majastrz
Copy link
Member

Looks like it was transient just unlucky because it just passed. Just merged it. 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.

bicep build: don't add empty template properties
3 participants