Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Add constantLine function #1730

Closed
wants to merge 11 commits into from
Closed

Conversation

agao48
Copy link
Contributor

@agao48 agao48 commented Mar 26, 2020

Describe your changes
Adds the constantLine function

Testing performed
Test files added with mtcmptest used locally

@replay
Copy link
Contributor

replay commented Mar 27, 2020

There are some formatting issues which make the circleCI tests fail

@kpfleming kpfleming deleted the constantLine branch March 27, 2020 20:57
schema.Point{Val: s.value, Ts: s.from},
schema.Point{Val: s.value, Ts: s.from + uint32((s.to-s.from)/2.0)},
schema.Point{Val: s.value, Ts: s.to},
)
Copy link
Contributor

Choose a reason for hiding this comment

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

it is guaranteed at this point (due to prior validation) that from < to.
however, from may equal to -1, which would result in duplicate timestamps. this is an edge case, and causes graphite to crash, but we may as well handle it :)

Copy link
Contributor

@Dieterbe Dieterbe left a comment

Choose a reason for hiding this comment

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

looks pretty good. just one comment, and as the CI message says, you'll need run gofmt.

You might want to run something like find . -name '*.go' -not -path './vendor/*' | xargs gofmt -w -s, which takes care of everything

@agao48
Copy link
Contributor Author

agao48 commented Mar 30, 2020

I added bloomberg@38e1c21 10 minutes ago, but not showing up here. There was some wonky stuff that happened over weekend where the original branch got deleted, so if the commit doesn't show up here by EOD, I'll close this PR and make a new one.

@agao48
Copy link
Contributor Author

agao48 commented Mar 30, 2020

closing per comment above ^ - will create new PR

@agao48 agao48 closed this Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants