Skip to content

Conversation

mcasimir
Copy link
Collaborator

@mcasimir mcasimir commented Jan 26, 2022

Screenshot 2022-01-26 at 17 55 33

Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

lgtm!

export const GSSAPI_PRINCIPAL_NAME_LABEL = 'Principal';
export const GSSAPI_SERVICE_NAME_LABEL = 'Service Name';
export const GSSAPI_CANONICALIZE_HOST_NAME_LABEL = 'Canonicalize Host Name';
export const GSSAPI_SERVICE_REALM_LABEL = 'Service Realm';
Copy link
Member

Choose a reason for hiding this comment

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

This might be a totally unnecessary thing I do, but with things like label names I like to not export them for tests if possible and instead make them hardcoded strings. It does mean that every time a label is updated we need to update the test, but I think that helps make the test a bit more solid and regression safe if the label accidentally has a typo added or something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's a good point, i'll revert everything in the next one :)

@mcasimir mcasimir merged commit a867bab into main Jan 26, 2022
@mcasimir mcasimir deleted the add-aws-auth branch January 26, 2022 17:37
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.

3 participants