search

MicrosoftDocs / azure-devops-docs

This repo is the home of the official Azure DevOps documentation for Microsoft. GitHub Issues filed in this repository should be for problems with the documentation.
https://docs.microsoft.com/azure/devops/
Creative Commons Attribution 4.0 International
830 stars 2.45k forks source link

Make KubernetesManifest actions less confusing #7841

Closed sliekens closed 3 years ago

sliekens commented 4 years ago

Instead of repeating "acceptable values" for action in every table, just mention the action name and its options once per table.

PRMerger9 commented 4 years ago

@StevenLiekens : Thanks for your contribution! The author(s) have been notified to review your proposed change.

PRMerger13 commented 4 years ago

@StevenLiekens : Thanks for your contribution! The author(s) have been notified to review your proposed change.

ktoliver commented 3 years ago

@azooinmyluggage - Hi, would you review the proposed revisions to your article? Thanks.

ktoliver commented 3 years ago

Hi @azooinmyluggage - Could you take a look at the updates proposed in this pull request? There's a file-level merge conflict--I can't spot the reason for it, but the pull request can't be merged. Do you have feedback about the proposed changes? Thanks.

juliakm commented 3 years ago

@StevenLiekens I'm sorry for the delay on this. I'm trying to understand this PR further. From looking at the task code, it seems like the options listed are valid. What if instead or removing the default values you added a line in each table that says something like "Use bake for the Bake action"?

sliekens commented 3 years ago

@juliakm the diff doesn't really show what is happening but essentially the current page layout is this:

## Deploy action

| Parameter | Description |
| ------------|--------------|
| Action | Acceptable values are <b>deploy</b>, <b>promote</b>, <b>reject</b>, <b>bake</b>, <b>scale</b>, <b>patch</b>, and <b>delete</b>.</td>|
| SomeDeployParameter | Required if action is set to <b>deploy</b> |

## Bake action

| Parameter | Description |
| ------------|--------------|
| Action | Acceptable values are <b>deploy</b>, <b>promote</b>, <b>reject</b>, <b>bake</b>, <b>scale</b>, <b>patch</b>, and <b>delete</b>.</td>|
| SomeBakeParameter | Required if action is set to <b>bake</b> |

My suggestion is that each table should only list stuff that is relevant to the current heading.

## Deploy action

| Parameter | Description |
| ------------|--------------|
| Action | <b>deploy</b>|
| SomeDeployParameter | Required |

## Bake action

| Parameter | Description |
| ------------|--------------|
| Action | <b>bake</b> |
| SomeBakeParameter | Required |
juliakm commented 3 years ago

sign-off

Thanks @StevenLiekens! I see what you mean now. I'm going to approve the PR and we can see if there is a need for updates later on.

PRMerger9 commented 3 years ago

@juliakm: I'm sorry - only the author of this article, @azooinmyluggage, can sign off on your changes. But we do have an exception process - if you are on the Microsoft content or product team for this product area, you can ask the PR review team to review and merge it by sending mail to the techdocprs alias.

sliekens commented 3 years ago

Careful, I didn't keep this up to date with changes to the main branch so this content is very stale. I can restart the merge if everyone agrees that it will make the documentation better.

ktoliver commented 3 years ago

@StevenLiekens I'm looking at the merge conflict right now. GitHub indicates that the pull request can't be merged in easily; manual adjustments would need to be made to fit your updates into the article.

If you would, could you create a new pull request against the current state of master, and we'll get the approved updates merged in? Thanks very much.