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

[Modules] Updated DeploymentExamples title & moved Bicep in front of JSON #1632

Merged
merged 47 commits into from
Jul 15, 2022

Conversation

AlexanderSehr
Copy link
Contributor

@AlexanderSehr AlexanderSehr commented Jul 1, 2022

Description

  • Updated DeploymentExamples title (now also showing the title of the file, instead of only a number)
  • Moved Bicep in front of JSON
  • Excluded new linter rule (which demands to define properties not with quotes where possible)

TODO:

  • Underneath deployment examples header add a disclaimer: What do the example titles refer to, parameters are ordered, where is the cut to optional, etc.
  • In the examples themselves (where is the split)

Pipeline references

For module/pipeline changes, please create and attach the status badge of your successful run.

Pipeline
AnalysisServices: Servers

Type of Change

Please delete options that are not relevant.

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Update to documentation

@AlexanderSehr AlexanderSehr marked this pull request as ready for review July 1, 2022 11:56
@AlexanderSehr AlexanderSehr requested a review from a team as a code owner July 1, 2022 11:56
@AlexanderSehr AlexanderSehr added documentation Improvements or additions to documentation enhancement New feature or request [cat] modules category: modules labels Jul 1, 2022
@AlexanderSehr AlexanderSehr enabled auto-merge (squash) July 1, 2022 11:58
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

Unit Test Results

  1 files  ±    0    1 suites  ±0   14s ⏱️ -51s
42 tests  - 358  42 ✔️  - 350  0 💤  - 8  0 ±0 
44 runs   - 357  44 ✔️  - 349  0 💤  - 8  0 ±0 

Results for commit 7d03399. ± Comparison against base commit 013bc47.

♻️ This comment has been updated with latest results.

@AlexanderSehr AlexanderSehr marked this pull request as draft July 5, 2022 20:29
auto-merge was automatically disabled July 5, 2022 20:29

Pull request was converted to draft

@AlexanderSehr AlexanderSehr marked this pull request as ready for review July 5, 2022 21:56
@AlexanderSehr AlexanderSehr enabled auto-merge (squash) July 5, 2022 21:59
@AlexanderSehr
Copy link
Contributor Author

AlexanderSehr commented Jul 7, 2022

🍰

AlexanderSehr and others added 3 commits July 8, 2022 21:09
@AlexanderSehr AlexanderSehr enabled auto-merge (squash) July 8, 2022 19:13
@AlexanderSehr AlexanderSehr requested a review from eriqua July 8, 2022 19:14
AlexanderSehr and others added 4 commits July 10, 2022 23:49
Co-authored-by: Erika Gressi <56914614+eriqua@users.noreply.github.com>
Co-authored-by: Erika Gressi <56914614+eriqua@users.noreply.github.com>
@itpropro
Copy link
Contributor

Just adding my question from #1655 to have the discussion here.

Why are we not complying with Bicep best practices for the linter rule? When just importing some of the modules in a project, the current bicepconfig.json is probably not going to be copied or the project has it's own config. Also how does this behave with CARML modules consumed from a ACR?
I think we should comply with the Bicep linter wherever possible, what is the benefit in deviating from the linter rule, if it has downsides for all CARML customers that will receive warnings in their projects.
The linter rule itself adjusts the syntax to other languages like JavaScript.

@AlexanderSehr
Copy link
Contributor Author

AlexanderSehr commented Jul 11, 2022

Just adding my question from #1655 to have the discussion here.

Why are we not complying with Bicep best practices for the linter rule? When just importing some of the modules in a project, the current bicepconfig.json is probably not going to be copied or the project has it's own config. Also how does this behave with CARML modules consumed from a ACR? I think we should comply with the Bicep linter wherever possible, what is the benefit in deviating from the linter rule, if it has downsides for all CARML customers that will receive warnings in their projects. The linter rule itself adjusts the syntax to other languages like JavaScript.

Replying similar to the comment in the other PR - let's rather do it all at ones in a PR designed for it, rather then a fractured way ;) #1664

bicepconfig.json Outdated Show resolved Hide resolved
AlexanderSehr and others added 2 commits July 15, 2022 15:44
Co-authored-by: Erika Gressi <56914614+eriqua@users.noreply.github.com>
Copy link
Contributor

@eriqua eriqua left a comment

Choose a reason for hiding this comment

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

🆗

@AlexanderSehr AlexanderSehr merged commit fac5bcc into main Jul 15, 2022
@AlexanderSehr AlexanderSehr deleted the users/alsehr/readMeTitleUpdate branch July 15, 2022 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[cat] modules category: modules documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants