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 work item tag cmdlets to Get, Remove and Update, also fixed typos and casing #418

Merged
merged 16 commits into from
Jan 11, 2022

Conversation

smholvoet
Copy link
Contributor

@smholvoet smholvoet commented Sep 17, 2021

PR Summary

Fixed casing and a couple of minor typos.

Update by Maintainer (@SebastianSchuetze): New cmdlets where added by @smholvoet

Suggestion

Another suggestion (which I have not yet pushed to my branch) is reworking the Notes section. I personally think it's very annoying seeing relevant cmdlets mentioned there, which are not clickable. Granted, they are mentioned a bit further down in the Related links section, but I think making the cmdlets clickable in the Notes section prevents a user from having to scroll further down and find the exact cmdlet he was looking for in a list of (sometimes) many more cmdlets:

image

I've noticed some cmdlets have related links which get added below the cmdlets which are displayed through the prerequisites.md file, for example Update-VSTeamPolicy, which I think is very useful.

In summary:

  • Remove <!-- #include "./common/related.md" --> from the Related links section
  • Rework the Notes section to include link, making things easier for users

Let me know what you think and I'll push additional changes to this branch should you agree with the above suggestion.

PR Checklist

@SebastianSchuetze
Copy link
Collaborator

SebastianSchuetze commented Sep 18, 2021

Another suggestion (which I have not yet pushed to my branch) is reworking the Notes section. I personally think it's very annoying seeing relevant cmdlets mentioned there, which are not clickable. Granted, they are mentioned a bit further down in the Related links section, but I think making the cmdlets clickable in the Notes section prevents a user from having to scroll further down and find the exact cmdlet he was looking for in a list of (sometimes) many more cmdlets:

image

@smholvoet Are you talking about the github docs pages regarding the links of the source markdown pages?

@smholvoet
Copy link
Contributor Author

@SebastianSchuetze I'm indeed talking about the docs page, for example: Get-VSTeamAccounts.

So in short:

  1. Remove the fixed commands which get added to the Related links section, which has been added to every command:
    image

  2. Rework the Notes section to include direct links to any cmdlet that gets mentioned there.

@SebastianSchuetze
Copy link
Collaborator

@DarqueWarrior what was the intention to add the standard related texts in every cmdlet? Is this nessecary to mention it everywhere?

@SebastianSchuetze
Copy link
Collaborator

@smholvoet you can remove the related part. You are right, that it may be deserve it's own page once instead of having it on every cmdlet.

@SebastianSchuetze I'm indeed talking about the docs page, for example: Get-VSTeamAccounts.

So in short:

  1. Remove the fixed commands which get added to the Related links section, which has been added to every command:
    image
  2. Rework the Notes section to include direct links to any cmdlet that gets mentioned there.

Copy link
Collaborator

@SebastianSchuetze SebastianSchuetze left a comment

Choose a reason for hiding this comment

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

Checked your current code.

Since unit tests are failing, then I guess you are not finished yet.

.docs/Get-VSTeamWorkItemTag.md Outdated Show resolved Hide resolved
.docs/Remove-VSTeamWorkItemTag.md Outdated Show resolved Hide resolved
.docs/Update-VSTeamWorkItemTag.md Outdated Show resolved Hide resolved
Source/Public/Get-VSTeamWorkItemTag.ps1 Show resolved Hide resolved
Source/Public/Remove-VSTeamWorkItemTag.ps1 Outdated Show resolved Hide resolved
Source/Public/Update-VSTeamWorkItemTag.ps1 Outdated Show resolved Hide resolved
Tests/function/tests/Remove-VSTeamWorkItemTag.Tests.ps1 Outdated Show resolved Hide resolved
@SebastianSchuetze SebastianSchuetze changed the title ✏️ fixed casing and minor typos ✏️ Added work item tag cmdlets to Get, Remove and Update, also fixed typos and casing Oct 4, 2021
@SebastianSchuetze SebastianSchuetze linked an issue Oct 10, 2021 that may be closed by this pull request
@SebastianSchuetze SebastianSchuetze added the enhancement Improvements that do not include new features label Oct 10, 2021
@SebastianSchuetze
Copy link
Collaborator

@smholvoet any update on your PR?

@smholvoet
Copy link
Contributor Author

@SebastianSchuetze I'll rework this in the next couple of days, thank you for reviewing 👌

Can I go ahead and rework the "related links" section as well? This would impact 172 files (all cmdlets).
Or should I hold off on this until @DarqueWarrior's response clears this up? 🤔

@DarqueWarrior what was the intention to add the standard related texts in every cmdlet? Is this nessecary to mention it everywhere?

@SebastianSchuetze
Copy link
Collaborator

No it's fine. You can rework. Please just do it in an extra PR.

Thanks for the support. Appreciate it!

@SebastianSchuetze
Copy link
Collaborator

@smholvoet got any time to finish it? :-)

@SebastianSchuetze
Copy link
Collaborator

@smholvoet thanks for you work! Please also check the builds. Your unit tests do not seem to go through. Without this we cannot merge.

image

Copy link
Collaborator

@SebastianSchuetze SebastianSchuetze left a comment

Choose a reason for hiding this comment

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

Just saw the file docs/synopsis/Update-VSTeamWorkItemTag.md

Synopsis is missing there.

You can also run the unit tests locally to check if they work.

https://github.com/MethodsAndPractices/vsteam/blob/trunk/README.md#how-to-build-locally

Copy link
Collaborator

@SebastianSchuetze SebastianSchuetze left a comment

Choose a reason for hiding this comment

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

Made quite some changes to the unit test. I guess you didn't run them at all locally. :-)

But really thank you for introducing this. I need to check a view more things, then I can merge this.

@SebastianSchuetze SebastianSchuetze merged commit f2772aa into MethodsAndPractices:trunk Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements that do not include new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No cmdlets to manage work item tags?
2 participants