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

Updated readme #102

Closed
wants to merge 1 commit into from
Closed

Conversation

arnaud-tincelin
Copy link
Collaborator

No description provided.

Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

There are few changes which takes the repo move away from early working contract and this PR removes some good documentation we need for those scenarios.

We need to discuss especially this is moving away from one of very key AKS use case.

Like I mentioned in one of my comment:

🐛 Why is this removed? Reason? (Like mentioned few times primary contract of this tool is to coexist with az-cli and vscode so lets sync up a meeting with @qpetraroia and @rzhang628 - but we cannot remove this, Collect command is one of the key use for this and supports few AKS scenarios.


0. If CLI extension aks-preview has been installed previously, uninstall it first.
Copy link
Member

Choose a reason for hiding this comment

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

🐛 Why is this removed? Reason? (Like mentioned few times primary contract of this tool is to coexist with az-cli and vscode so lets sync up a meeting with @qpetraroia and @rzhang628 - but we cannot remove this, Collect command is one of the key use for this and supports few AKS scenarios.

Copy link
Collaborator Author

@arnaud-tincelin arnaud-tincelin Aug 12, 2021

Choose a reason for hiding this comment

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

yes but the kollect doc shall be in the kollect repo. This documentation should be moved. It's not aks-periscope reponsability to know how external tools use it.
Furthermore, to maintain this documentation up-to-date it would mean to watch for new releases of kollect command and update our readme accordingly - this is painful

It's not moving away from any scenario. It's just refocusing on what it's doing: it's a tool to run diagnostics; not a command line

I think we should also mention the integration with vscode which is not documented at all at the moment (no link to the extension repo)

Copy link
Member

@Tatsinnit Tatsinnit Aug 22, 2021

Choose a reason for hiding this comment

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

💡 I have blocked some time this week, might be there is missing information or could be background information, hence you probably are making these changes, the kollect command was written at the same time and was the idea behind this tool. Hence in various places including the internals have updated a readMe file regarding consuming tools here: https://github.com/Azure/aks-periscope#dependent-consuming-tools-and-working-contract

I would recommend that we can improve the documentation by proper - Wiki pages. Which might also include the consensus on other things. Removing the key usability docs from the documentation is a bad idea. We can discuss this in tomorrows catch-up (if possible) leaving this for wider PM visibility.

Copy link
Collaborator Author

@arnaud-tincelin arnaud-tincelin Aug 23, 2021

Choose a reason for hiding this comment

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

Don't you think it's weird to have the code in one repo and documentation in another one, managed by another team? That's why I suggested to put code & doc at the same place. If we agree on this current PR, all doc we remove from here, we shall put it in a PR on the kollect repo and add a link to this repo from our readme. In my opinion, that would make more sense.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, @rzhang628 - has an idea for workitem to handle this in wiki for this repo. Which sounds like a great plan and thanks for that.

@Tatsinnit
Copy link
Member

closing this as the new read me changes reside here: #186 ❤️☕️🙏

@Tatsinnit Tatsinnit closed this Jun 29, 2022
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.

2 participants