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

v0.0.1 Release #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CurtisLusmore
Copy link

Added the following key functions:

  • New-AwsAccount
  • Edit-AwsAccount
  • New-AwsRole
  • Set-AwsProfile
  • New-AwsSession

Added the following key functions:
* New-AwsAccount
* Edit-AwsAccount
* New-AwsRole
* Set-AwsProfile
* New-AwsSession
@CurtisLusmore
Copy link
Author

I should update the README as well

)

# Cmdlets to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no cmdlets to export.
CmdletsToExport = @()

Choose a reason for hiding this comment

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

What's the difference between CmdletsToExport and FunctionsToExport?

Copy link
Author

Choose a reason for hiding this comment

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

I... don't know. Honestly I barely know the difference between a function and a cmdlet.

Choose a reason for hiding this comment

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

My understanding is that a CmdLet is a Function with special powers... when you add the attribute [CmdletBinding()] This turns an ordinary function into a commandlet that can support things like -Verbose -WhatIf etc... without the attribute, it's just a plain old Function.

Copy link
Contributor

Choose a reason for hiding this comment

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

in the module manifest the CmdletsToExport refers to binary CMDlets that have been compiled from C# and available in a linked dll.

The FunctionsToExport is for script functions regardless of whether they do or do not have the [CMDletBinding()]or not

# Tags = @()

# A URL to the license for this module.
# LicenseUri = ''

Choose a reason for hiding this comment

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

Have you considered which open source license to use? Is this something that TP will dictate, or can you choose?

Copy link
Author

Choose a reason for hiding this comment

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

Good question, I'll ping some peeps and see what we should use. MIT would be nice.

Copy link
Contributor

Choose a reason for hiding this comment

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

We use MIT for our existing powershell modules and is the preferred license in general

A collection of functions for managing AWS CLI config and credentials.

.DESCRIPTION
A collection of functions for managing AWS CLI config and credentials.

Choose a reason for hiding this comment

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

Might want to dive a bit deeper in the description.

Copy link
Author

Choose a reason for hiding this comment

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

Yep good idea. I'll be filling out the README too so likely to be a lot of overlap in content here.

or CSV file. This will silently ignore if the account already exists. To edit
an existing account, use Edit-AwsAccount.

.PARAMETER AccountName

Choose a reason for hiding this comment

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

The naming might need some work. From an AWS perspective the concept of an Account is very well defined, but this does not map one-to-one to an "AWS Account". Not sure what other naming you could use, maybe Organisation, or Realm or Identity???? I guess this comment also applies to the Function name too. It may just be a case of it' too hard to name it anything other than Account.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I reckon Identity or User would do the trick.

PwshAwsConfig.psm1 Show resolved Hide resolved
PwshAwsConfig.psm1 Show resolved Hide resolved
PwshAwsConfig.psm1 Show resolved Hide resolved
Function New-AwsAccount
{
Param(
[Parameter(Mandatory)] [string] $AccountName,

Choose a reason for hiding this comment

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

One thing I like to do in scripts that will be used a lot is to make them positional parameters where possible (e.g. [Parameter(Mandatory=$true, Position=0)] etc...), that way a power user could just go New-AwsAccount vicroads accesskeys.csv rather than having to type in out the parameter names in full to disambiguate.

Copy link
Author

Choose a reason for hiding this comment

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

Happy to add this explicitly, although I think this Just Works™ naturally by defaulting to the order listed.

Param(
[string] $Profile = $env:AWS_PROFILE,
[string] $Code,
[string] $Arn

Choose a reason for hiding this comment

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

Same comment about positional parameters as above


# Modules to import as nested modules of the module specified in RootModule/ModuleToProcess
NestedModules = @(
'PwshAwsConfig.psm1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Typically the psm1 module file is added under RootModule and NestedModules is used for libraries that you bundle together with your module.

Copy link
Author

Choose a reason for hiding this comment

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

Cool thanks, fixed!

NewOrEdit-AwsAccount -AccountName $AccountName -AccessKeyCsvPath $AccessKeyCsvPath
}

<#
Copy link
Contributor

Choose a reason for hiding this comment

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

I think comment based help needs to be within the function block before the Param(bit in order to surface properly. You can check by loading the module and calling Get-Help Edit-AwsAccount

Copy link
Author

Choose a reason for hiding this comment

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

Wow interesting, I had only ever seen it like this, before the Function keyword. It turns out it can go in 3 places!

https://docs.microsoft.com/en-us/powershell/scripting/developer/help/placing-comment-based-help-in-functions?view=powershell-7.1


If (Test-AwsProfile -Profile $credentialName)
{
Write-Information "Account already exists"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe reword to Profile Already Exists

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.

4 participants