-
Notifications
You must be signed in to change notification settings - Fork 165
ReadSettings output JSON structured based on get input parameter. #1882
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new feature that consolidates the output of selected settings from the ReadSettings action into a single JSON structure, streamlining how workflow outputs are handled. Instead of requiring separate steps to extract individual settings and add them to job outputs, all settings specified in the get
parameter are now available in a compressed JSON format through the SelectedSettingsJson
output.
- Adds
SelectedSettingsJson
output to ReadSettings action containing all selected settings in JSON format - Removes dedicated workflow steps for extracting individual settings like
DetermineWorkflowDepth
andDeterminePowerPlatformSolutionFolder
- Updates all workflow templates to use the new JSON output format with
fromJson()
function
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Actions/ReadSettings/ReadSettings.ps1 | Implements collection and JSON serialization of selected settings |
Actions/ReadSettings/action.yaml | Adds SelectedSettingsJson output definition |
Actions/ReadSettings/README.md | Documents the new SelectedSettingsJson output |
Tests/ReadSettings.Action.Test.ps1 | Adds comprehensive test coverage for the new JSON output functionality |
Templates/.../workflows/*.yaml | Updates workflow templates to use new JSON output format and removes redundant steps |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
workflowDepth: ${{ env.workflowDepth }} | ||
powerPlatformSolutionFolder: ${{ fromJson(steps.ReadSettings.outputs.SelectedSettingsJson).powerPlatformSolutionFolder }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can just assign workflowDepth to ${{ env.workflowDepth }} couldn't we do the same for powerPlatformSolutionFolder.
The settings defined in the "get" parameter are already assigned to an environment variable AFAIK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only works for workflowDepth because it's a global environment variable defined on line 23. The environment variables created by readsettings for things in the get param can't be used for job outputs, hence the need for putting everything in "get" into a separate output variable which can then be used for the job output.
That said, it seems the workflowDepth output is not actually used. In this PR I kept it for now, but we can probably remove it completely as the whole workflow should have access to env.workflowDepth given it's defined on the global level.
❔What, Why & How
This feature changes how we handle output variables from ReadSettings. The current solution is to add a setting to the get parameter of ReadSettings, and then add a step like this:
This is then added to the outputs of a job like this:
This new solution takes all settings selected in the get parameter of ReadSettings and puts them in a single JSON structure. This can then be used directly in the job outputs like this:
✅ Checklist