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

removed clear_heading while sanitizing in loki_worker_t #4288

Merged

Conversation

jehamannco
Copy link
Contributor

@jehamannco jehamannco commented Sep 5, 2023

Issue

This PR targets issue #1802. Here the heading information was erased during matrix calculation. This aim to keep the heading data.

Tasklist

  • Add tests
  • Add #fixes with the issue number that this PR addresses
  • Update the docs with any new request parameters or changes to behavior described
  • Update the changelog
  • If you made changes to the lua files, update the taginfo too.

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@nilsnolde
Copy link
Member

Seems the tests pass at least. Unless someone has objections, I'd merge this. @dnesbitt61 or @kevinkreiser ?

Can you also update the changelog in the BUG FIX section @jehamannco ? New items go to the bottom of the list.

@dnesbitt61
Copy link
Member

Seems fine to me. I've used this in our fork of Valhalla for quite a while with one to many matrices, so no objection from me.

Copy link
Member

@nilsnolde nilsnolde left a comment

Choose a reason for hiding this comment

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

LGTM and no test needed

@nilsnolde nilsnolde merged commit 97cad06 into valhalla:master Sep 13, 2023
8 checks passed
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.

None yet

3 participants