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

Use fork repo for daily updates in Java #25677

Merged
merged 14 commits into from
Dec 2, 2021
Merged

Conversation

sima-zhu
Copy link
Contributor

@sima-zhu sima-zhu commented Nov 24, 2021

Description

The PR is to update the version to docs fork repo daily branch.
Keyvault testing:
From unified pipeline to fork repo:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1217760&view=logs&j=b1e79959-24d8-5aa9-2799-72d40c3e051b&t=77783793-89a5-5b26-f964-83f7ae88ef10
https://github.com/azure-sdk/azure-docs-sdk-java/blob/daily/2021-11-29/metadata/preview/azure-security-keyvault-administration.json#L4

From azure-sdk fork to main daily branch:
As we have unified pipeline and ci build run to override the changes. I have screenshot the changes in main repo.
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1217885&view=results
image
Azure/azure-docs-sdk-java@a60c4e7

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@sima-zhu sima-zhu requested a review from danieljurek November 29, 2021 18:48
- pwsh: |
$dailyDocsRepoLocation = "$(DocRepoLocation)/dailyDocs"
echo "##vso[task.setvariable variable=DailyDocsRepoLocation]$dailyDocsRepoLocation"
if (!(Test-Path $dailyDocsRepoLocation)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to create this? Won't the clone create it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The working directory contains two clones, 1 main branch from Azure repo, 2 daily branch from azure-sdk repo.
1 used in schedule run updating the Azure main repo. (For release ones)
2 used in schedule run updating the Azure daily repo. (For daily docs)

Both branches get updated in schedule run.

Copy link
Member

Choose a reason for hiding this comment

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

We can consider using the same clone but just adding a new remote that would save on some of the forking going on and the duplicate cloning.

@@ -49,13 +49,30 @@ jobs:
- template: /eng/common/pipelines/templates/steps/set-daily-docs-branch-name.yml
parameters:
DailyBranchVariableName: DailyDocsBranchName
# Sync docs fork repo onboarding files/folders
- pwsh: |
$dailyDocsRepoLocation = "$(DocRepoLocation)/dailyDocs"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to set this dynamically or can we set it statically where we define DocRepoLocation?

Copy link
Contributor Author

@sima-zhu sima-zhu Nov 29, 2021

Choose a reason for hiding this comment

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

In DocRepoLocation ,

Pull remote Azure/main -> local Azure/main 
Push local Azure/main -> remote Azure/main

In dailyDocsRepoLocation ,

Pull remote azure-sdk/daily-> local azure-sdk/daily
Push local azure-sdk/daily-> remote Azure/daily

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two steps both happen in schedule run

Copy link
Member

Choose a reason for hiding this comment

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

I think we can eliminate this pwsh task by setting the variable above:

    variables:
      DocRepoLocation: $(Pipeline.Workspace)/docs
      DailyDocRepoLocation: $(Pipeline.Workspace)/dailyDocsFork  # Add this line
      DocRepoOwner: Azure
      DocRepoName: azure-docs-sdk-java

That would eliminate the need to create this variable at runtime and the sparse checkout process handles creating the directory.

Copy link
Member

Choose a reason for hiding this comment

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

Yes that is wat I was wondering about. However see my other comment as well as we should consider just adding a remote to the fork instead of a separate clone.

@sima-zhu sima-zhu requested a review from weshaggard November 29, 2021 22:01
@danieljurek
Copy link
Member

These changes and the runs look correct. I think we can eliminate some redundant complexity and this should be good to go 👍

Copy link
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

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

Approved.

This shouldn't hold up the merge but we can also remove TargetDocRepoName from the release template.

@sima-zhu sima-zhu requested a review from weshaggard December 1, 2021 18:22
@sima-zhu
Copy link
Contributor Author

sima-zhu commented Dec 1, 2021

@sima-zhu sima-zhu requested a review from weshaggard December 1, 2021 21:47
@sima-zhu sima-zhu merged commit bee5b9b into Azure:main Dec 2, 2021
@sima-zhu sima-zhu deleted the fork_branch branch December 2, 2021 18:44
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this pull request Sep 12, 2023
Add 2023-09-01-preview for appplatform (Azure#25501)

* Adds base for updating Microsoft.AppPlatform from version preview/2023-07-01-preview to version 2023-09-01-preview

* Updates readme

* Updates API version in new specs and examples

* feat: fragment support swagger change (Azure#25314)

* Add apiTryOutEnabledState properties for api portal for 2023-09-01-preview api (Azure#25322)

* support war file deployment (Azure#25312)

Co-authored-by: Yi Li <yili7@microsoft.com>

* Fix exmaple for api portal api try-out feature (Azure#25337)

* Add property `autoSync` in `KeyVaultCertificateProperties` (Azure#25331)

* Add gateway apm reference (Azure#25408)

* Add expand parameter for list deployment (Azure#25411)

* Add expand parameter for list deployments API (Azure#25471)

* change order of expand (Azure#25587)

* Update readme.go.md (Azure#25634)

* Update readme.go.md

* Update readme.python.md

* Update readme.ruby.md

* Revert api portal try out feature (Azure#25677)

Fix exmaple for api portal api try-out feature (Azure#25337)"

* add API to list all server versions (Azure#25720)

* add API to list all server versions

* refine list supported server version API

* refine description

---------

Co-authored-by: Yi Li <yili7@microsoft.com>

* use enum for autoSync (Azure#25722)

* Update appplatform.json (Azure#25724)

* certificate - rename enum (Azure#25723)

* use enum for autoSync

* rename enum

* rename enum

---------

Co-authored-by: guitarsheng <85543793+guitarsheng@users.noreply.github.com>
Co-authored-by: Mason Chen <jiec@microsoft.com>
Co-authored-by: Yi Li <109205537+yilims@users.noreply.github.com>
Co-authored-by: Yi Li <yili7@microsoft.com>
Co-authored-by: Jeff <1830237+domainname@users.noreply.github.com>
Co-authored-by: ninpan-ms <71061174+ninpan-ms@users.noreply.github.com>
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.

3 participants