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

Adds -Credential parameter and Truncate Warning #15

Merged
merged 13 commits into from
Sep 7, 2024
Merged

Conversation

PoshAJ
Copy link
Collaborator

@PoshAJ PoshAJ commented Sep 6, 2024

Closes #13 and #14

  • Update documentation for -Credential.
  • Test a643c27 in multi-domain environment.

@PoshAJ PoshAJ marked this pull request as draft September 6, 2024 16:58
@santisq santisq assigned santisq and PoshAJ and unassigned santisq Sep 6, 2024
@santisq santisq requested review from santisq and CrookedJ September 6, 2024 17:11
@santisq santisq added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 6, 2024
@santisq
Copy link
Owner

santisq commented Sep 6, 2024

@PoshAJ -Credential can only be used with -Server ? Is that correct ?

Copy link
Owner

@santisq santisq left a comment

Choose a reason for hiding this comment

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

I think these changes make sense given the cmdlets support pipeline input. With the previous logic it would've shown the warning message just once if I'm not mistaken.

src/PSADTree/PSADTreeCmdletBase.cs Outdated Show resolved Hide resolved
src/PSADTree/Commands/GetADTreeGroupMemberCommand.cs Outdated Show resolved Hide resolved
@PoshAJ
Copy link
Collaborator Author

PoshAJ commented Sep 6, 2024

@PoshAJ -Credential can only be used with -Server ? Is that correct ?

Yes, as it's currently designed. We could support it separately, but it's probably fine to require -Server.

@santisq
Copy link
Owner

santisq commented Sep 6, 2024

@PoshAJ -Credential can only be used with -Server ? Is that correct ?

Yes, as it's currently designed. We could support it separately, but it's probably fine to require -Server.

Ahhh because of the overloads, didn't check them 😅 Perhaps we should add a parameter set for -Credential only when -Server is used

@PoshAJ
Copy link
Collaborator Author

PoshAJ commented Sep 6, 2024

@PoshAJ -Credential can only be used with -Server ? Is that correct ?

Yes, as it's currently designed. We could support it separately, but it's probably fine to require -Server.

Ahhh because of the overloads, didn't check them 😅 Perhaps we should add a parameter set for -Credential only when -Server is used

Yeah, we can go a couple ways with this one?
1. Parameter Sets
2. Dynamic Parameter
3. Support -Credential without -Server
4. Throw ArgumentException

@PoshAJ PoshAJ changed the title WIP: Adds -Credential parameter and Truncate Warning Adds -Credential parameter and Truncate Warning Sep 7, 2024
@PoshAJ PoshAJ marked this pull request as ready for review September 7, 2024 17:49
@santisq santisq merged commit 2f01962 into main Sep 7, 2024
4 checks passed
@santisq santisq deleted the 13/truncate-warning branch September 7, 2024 18:01
@santisq santisq mentioned this pull request Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Truncate Warning
2 participants