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

Update prom libs #379

Merged
merged 4 commits into from
Jun 9, 2021
Merged

Update prom libs #379

merged 4 commits into from
Jun 9, 2021

Conversation

sagikazarmark
Copy link
Contributor

No description provided.

SuperQ
SuperQ requested changes Jun 9, 2021
View reviewed changes
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Some minor issues with the log library conversion.

pkg/mapper/fsm/fsm.go Outdated Show resolved Hide resolved
pkg/mapper/fsm/fsm.go Outdated Show resolved Hide resolved
pkg/mapper/fsm/fsm.go Outdated Show resolved Hide resolved
@sagikazarmark
Copy link
Contributor Author

Thanks! I applied the suggested changes.

@SuperQ
Copy link
Member

SuperQ commented Jun 9, 2021

Ahh, the fun of GitHub applying changes. You might want git rebase -i origin/master and squash the changes.

@SuperQ
Copy link
Member

SuperQ commented Jun 9, 2021

Looks like I made a typo. 😂

pkg/mapper/fsm/fsm.go:253:110: missing ',' in argument list (typecheck)
			level.Warn(logger).Log("msg", "Invalid match, cannot compile regex in mapping", "mapping", mapping, "err" err))
			                                                                                                          ^

@sagikazarmark @SuperQ
refactor: use structured logging
04bc13d 
Co-authored-by: Ben Kochie <[email protected]>
Signed-off-by: Mark Sagi-Kazar <[email protected]>
@sagikazarmark
Copy link
Contributor Author

Rebased, signed, typo fixed. Do you want me to squash as well?

SuperQ
SuperQ approved these changes Jun 9, 2021
View reviewed changes
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM, I'll leave it to @matthiasr to finish up.

@SuperQ SuperQ requested a review from matthiasr June 9, 2021 12:52
matthiasr
matthiasr approved these changes Jun 9, 2021
View reviewed changes
Copy link
Contributor

@matthiasr matthiasr left a comment

Choose a reason for hiding this comment

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

Thank you!

@matthiasr matthiasr merged commit 5ea682c into prometheus:master Jun 9, 2021
@sagikazarmark sagikazarmark deleted the update-prom-libs branch June 9, 2021 17:36
@sagikazarmark
Copy link
Contributor Author

Any chance you could release a new version with the new dependencies? Thanks!

matthiasr added a commit that referenced this pull request Jun 10, 2021
@matthiasr
Changelog & version bump for #379
ef6627b 
Signed-off-by: Matthias Rampke <[email protected]>
@matthiasr
Copy link
Contributor

done!

@sagikazarmark
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperQ SuperQ SuperQ approved these changes

@matthiasr matthiasr matthiasr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sagikazarmark @SuperQ @matthiasr