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.

Sign up for GitHub

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

Jump to bottom

Feat: add windows os.build attribute to host fingerprint #17576

Merged
merged 1 commit into from Jun 21, 2023
Merged

Feat: add windows os.build attribute to host fingerprint #17576

merged 1 commit into from Jun 21, 2023

Conversation

VishnuJin
Copy link
Contributor

@VishnuJin VishnuJin commented Jun 17, 2023 •
edited

fixes #16591

@hashicorp-cla
Copy link

hashicorp-cla commented Jun 17, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Jun 20, 2023
tgross
tgross requested changes Jun 20, 2023
View reviewed changes
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @VishnuJin! This is looking pretty close.

In addition to the comments I've left, can you do the following:

Thanks!

client/fingerprint/host.go Outdated Show resolved Hide resolved
client/fingerprint/host.go Outdated Show resolved Hide resolved
@tgross tgross moved this from Needs Triage to Triaging in Nomad - Community Issues Triage Jun 20, 2023
@VishnuJin VishnuJin force-pushed the add-os-build-fingerprint-to-windows branch from a410d87 to 2298ace Compare June 20, 2023 16:36
@VishnuJin
Copy link
Contributor Author

@tgross made the necessary changes, please have a look

Thanks

@VishnuJin VishnuJin requested a review from tgross June 20, 2023 16:39
tgross
tgross requested changes Jun 20, 2023
View reviewed changes
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Almost there!

client/fingerprint/host.go Outdated Show resolved Hide resolved
client/fingerprint/host_test.go Outdated Show resolved Hide resolved
website/content/docs/runtime/interpolation.mdx Outdated Show resolved Hide resolved
@VishnuJin VishnuJin force-pushed the add-os-build-fingerprint-to-windows branch from 2298ace to 1da185c Compare June 21, 2023 03:18
@VishnuJin VishnuJin requested a review from tgross June 21, 2023 03:24
@VishnuJin
Copy link
Contributor Author

@tgross I’ve made the necessary changes again, please have a look

tgross
tgross approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @VishnuJin!

This will ship in the upcoming Nomad 1.6.0

@VishnuJin
Copy link
Contributor Author

@tgross this my first ever attempt in go, thanks for the kind feedback and guidance.
super happy 😄

@tgross tgross merged commit 102f732 into hashicorp:main Jun 21, 2023
19 of 20 checks passed
Nomad - Community Issues Triage automation moved this from Triaging to Done Jun 21, 2023
@tgross tgross added this to the 1.6.0 milestone Jun 21, 2023
@tgross tgross mentioned this pull request Jun 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross tgross tgross approved these changes

Assignees
No one assigned
Labels
theme/client theme/fingerprint theme/platform-windows type/enhancement
Projects
Nomad - Community Issues Triage
Done
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.

Fingerprint Windows Build Number
3 participants
@VishnuJin @hashicorp-cla @tgross