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 switch to skip sync and generate script call #6305

Merged
merged 10 commits into from
Jun 9, 2023

Conversation

raych1
Copy link
Member

@raych1 raych1 commented Jun 8, 2023

Per the agreement with @m-nash, adding this switch in Process script to be used by dotnet automation script.

With this switch, dotnet can keep most of current script code when adopting new common scripts. Dotnet would also keep using dotnet build /t:GenerateCode to generate sdk code.

In CI pipeline we have consistent experience, TypeSpec-Project-Process.ps1 would be called by each language automation script to interact with tspconfig.yaml and create/update tsp-location.yaml.

@weshaggard , can you review?

@raych1 raych1 requested a review from konrad-jamrozik as a code owner June 8, 2023 05:38
@raych1 raych1 self-assigned this Jun 8, 2023
@raych1 raych1 requested review from weshaggard and benbp as code owners June 8, 2023 05:38
@raych1 raych1 requested a review from m-nash June 8, 2023 05:38
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

I don't mind having the option that can be used but I'm not sure I exactly understand why it is needed. Aren't the existing scripts already calling sync and generate? If so, why wouldn't they just let the process script handle that.

In any case the biggest value is to have a common script to parse the tspconfig and create/update the tsp-location file. So as long as we not duplicating that and parsing the tspconfig in any other place I'm good with this.

@weshaggard
Copy link
Member

Looks like your branch is missing the latest changes from your other PR #6293, so to fix the pipeline you will need to rebase your new changes on top of those older ones.

scbedd and others added 3 commits June 8, 2023 11:16
* add line offering additional detail
* remove extraneous newline
* Add new project `assets-maintenance-tool`. Implement `scan` functionality within. `backup` and `cleanup` still to come.
* Add integration tests for new assets-maintenance tool.
* Move original `asset-sync` powershell proto script into `tools/assets-automation/` folder
* Move `eng/scripts/python/assets-automation/` under `tools/assets-automation/assets-reporting/`
* Add ci.yml and tests.yml to invoke integration tests against the azure-sdk-assets-integration

Co-authored-by: Konrad Jamrozik <[email protected]>
@raych1
Copy link
Member Author

raych1 commented Jun 9, 2023

I don't mind having the option that can be used but I'm not sure I exactly understand why it is needed. Aren't the existing scripts already calling sync and generate? If so, why wouldn't they just let the process script handle that.

In any case the biggest value is to have a common script to parse the tspconfig and create/update the tsp-location file. So as long as we not duplicating that and parsing the tspconfig in any other place I'm good with this.

@weshaggard , dotnet build command invokes sync and generate script and we want to keep this rather than switching to call process script. Instead, Process script would be called in the dotnet automation script to interact with tspconfig.yaml and create/update tsp-location.yaml.

@raych1 raych1 requested review from scbedd, mikeharder and a team as code owners June 9, 2023 01:29
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

azure-sdk added a commit to Azure/azure-sdk-for-js that referenced this pull request Jun 9, 2023
@raych1 raych1 merged commit 591df79 into main Jun 9, 2023
@raych1 raych1 deleted the user/raych1/add-skip-flag branch June 9, 2023 02:14
minhanh-phan pushed a commit to minhanh-phan/azure-sdk-for-js that referenced this pull request Jun 12, 2023
@weshaggard
Copy link
Member

@weshaggard , dotnet build command invokes sync and generate script and we want to keep this rather than switching to call process script. Instead, Process script would be called in the dotnet automation script to interact with tspconfig.yaml and create/update tsp-location.yaml.

@raych1 how would they know which projects to invoke the dotnet build command on? Mostly I want to avoid needing to read the tspconfig.yaml file directly in any other places.

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.

5 participants