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.

Sign up for GitHub

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

Jump to bottom

Add auto discovery of parameter files when scanning a directory #297

Merged
merged 28 commits into from
Nov 2, 2022
Merged

Add auto discovery of parameter files when scanning a directory #297

merged 28 commits into from
Nov 2, 2022

Conversation

pim-simons
Copy link
Contributor

@pim-simons pim-simons commented Oct 27, 2022 •
edited

Description

This PR adds auto discovery of parameter files when using the analyze-directory command. The code checks if a parameters file with naming according to Microsofts naming convention is present and if so uses this file as the input for the parameters file.

Added a test AnalyzeDirectory_ValidInputValues_AnalyzesExpectedNumberOfFilesWithParameters to check if the parameters file is indeed used.

I figured I'd try to contribute and help out since this wasn't high in the priority list, if anything is missing or not up to standards coding wise let me know!

Closes #296.

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • Description of the pull request is clear and informative.
  • I have added myself to the 'assignees'.
  • I have added 'linked issues' if relevant.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@pim-simons pim-simons requested a review from a team as a code owner October 27, 2022 19:18
@pim-simons
Copy link
Contributor Author

Regarding I have added myself to the 'assignees'.
I can't add myself as an assignee, probably rights related 😉

@pim-simons
Copy link
Contributor Author

@microsoft-github-policy-service agree

@pim-simons pim-simons mentioned this pull request Oct 27, 2022
Open
@VeraBE
Copy link
Contributor

VeraBE commented Oct 27, 2022 •
edited

I'm thinking we probably want to do the same for analyze-template, would you mind adding that? Abstracting away the logic of the file search in a function

@pim-simons
Copy link
Contributor Author

I'm thinking we probably want to do the same for analyze-template, would you mind adding that? Abstracting away the logic of the file search in a function

I added the same functionality in the analyze-template as well, see 34c1492 (#297)

@anaismiller
Copy link
Contributor

Would you also be able to add to our documentation that it uses this pararmeters path? Thank you for adding this in @pim-simons !

Pim Simons added 2 commits October 31, 2022 11:54
check for .parameters*.json so we also support the parameters file …
b348f20 
…naming standard where environment are defined (for example `azuredeploy.parameters-dev.json`)
update documentation
49eff74
@pim-simons
Copy link
Contributor Author

pim-simons commented Oct 31, 2022 •
edited

Would you also be able to add to our documentation that it uses this pararmeters path? Thank you for adding this in @pim-simons !

Thanks, happy to help 👍🏻
I updated the documentation, see 49eff74 (#297)

@VeraBE VeraBE self-requested a review October 31, 2022 17:05
@VeraBE VeraBE self-requested a review October 31, 2022 18:32
@pim-simons @VeraBE
Update logging according to code suggestion
a8c042a 
Co-authored-by: Vera Bogdanich Espina <VeraBE@users.noreply.github.com>
@VeraBE VeraBE self-requested a review November 1, 2022 18:32
VeraBE
VeraBE reviewed Nov 1, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
pim-simons and others added 4 commits November 1, 2022 19:39
@pim-simons @VeraBE
Update README.md
7e37cb2 
Co-authored-by: Vera Bogdanich Espina <VeraBE@users.noreply.github.com>
@pim-simons @VeraBE
Update src/Analyzer.Cli/CommandLineParser.cs
473ad37 
Co-authored-by: Vera Bogdanich Espina <VeraBE@users.noreply.github.com>
reorder usings
5718325
Merge branch 'parameters-auto-discovery' of https://github.com/pim-si…
d462194 
…mons/template-analyzer into parameters-auto-discovery
@VeraBE VeraBE self-requested a review November 1, 2022 18:55
Pim Simons and others added 7 commits November 1, 2022 19:56
@VeraBE VeraBE self-requested a review November 1, 2022 20:04
anaismiller
anaismiller approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@anaismiller anaismiller left a comment

Choose a reason for hiding this comment

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

Minor changes needed for me, approving!

README.md Outdated Show resolved Hide resolved
src/Analyzer.Cli/CommandLineParser.cs Outdated Show resolved Hide resolved
pim-simons and others added 4 commits November 2, 2022 07:51
@VeraBE VeraBE merged commit 5cc567d into Azure:development Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VeraBE VeraBE VeraBE approved these changes

@anaismiller anaismiller anaismiller approved these changes

Assignees

@pim-simons pim-simons

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE REQ] Support objects in parameters.json file
3 participants
@pim-simons @VeraBE @anaismiller