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

Restructuring generate-doc code folder #5252

Merged
merged 122 commits into from
Oct 25, 2019
Merged

Restructuring generate-doc code folder #5252

merged 122 commits into from
Oct 25, 2019

Conversation

KarishmaGhiya
Copy link
Member

  • Collapsing all the required dependencies for scripts to one package.json

danieljurek
danieljurek reviewed Oct 8, 2019
View reviewed changes
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.

Looks good. Putting DocArgs up in the parameters block is important for keeping our templates clean and readable. The rest are suggestions.

eng/pipelines/templates/stages/cosmos-sdk-client.yml Outdated Show resolved Hide resolved
eng/pipelines/templates/jobs/archetype-sdk-client.yml Outdated Show resolved Hide resolved
eng/pipelines/templates/stages/archetype-sdk-client.yml Outdated Show resolved Hide resolved
eng/tools/generate-doc/index.js Outdated Show resolved Hide resolved
eng/tools/generate-doc/index.js Outdated Show resolved Hide resolved
eng/tools/compress-subfolders.ps1 Outdated Show resolved Hide resolved
@HarshaNalluru HarshaNalluru mentioned this pull request Oct 16, 2019
Closed
2 tasks
@KarishmaGhiya KarishmaGhiya force-pushed the typedocFeatures branch 2 times, most recently from 7826e2d to ad3ac8a Compare October 23, 2019 18:53
weshaggard
weshaggard reviewed Oct 24, 2019
View reviewed changes
eng/pipelines/doc-index.yml
Copy-Item "$(Build.SourcesDirectory)/eng/tools/generate-static-index/static-files/docfx.json" -Destination "$(Build.SourcesDirectory)/docfx_project/" -Force
displayName: Copy over docfx.json

- script: |
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you are doing a lot of one line script or pswh commands you should consider consolidating them into a smaller number of pwsh or script steps.

Copy link
Member Author

Choose a reason for hiding this comment

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

So the way we have them right now is we have segregated the steps based on the logical purpose and also if there are any failures we can identify them easily

danieljurek
danieljurek requested changes Oct 24, 2019
View reviewed changes
eng/tools/generate-doc/index.js Outdated Show resolved Hide resolved
eng/tools/generate-doc/index.js Outdated Show resolved Hide resolved
weshaggard
weshaggard reviewed Oct 24, 2019
View reviewed changes
sdk/keyvault/ci.yml Outdated Show resolved Hide resolved
@KarishmaGhiya KarishmaGhiya merged commit 211d751 into Azure:master Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scbedd scbedd scbedd left review comments

@danieljurek danieljurek danieljurek approved these changes

@weshaggard weshaggard weshaggard approved these changes

@mikeharder mikeharder Awaiting requested review from mikeharder

@mitchdenny mitchdenny Awaiting requested review from mitchdenny

@michaeljqzq michaeljqzq Awaiting requested review from michaeljqzq

@sophiajt sophiajt Awaiting requested review from sophiajt

@sadasant sadasant Awaiting requested review from sadasant

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KarishmaGhiya @danieljurek @weshaggard @scbedd