Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: no job start time case #89

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Conversation

jerr
Copy link
Contributor

@jerr jerr commented Sep 17, 2019

In some cases, the job has no start time (Job.Status.StartTime = nil).
We keep the pointer values ​​and display <unknown> in the case of nil thanks to timestamp.IsZero()

func (t *Time) IsZero() bool {
	if t == nil {
		return true
	}
	return t.Time.IsZero()
}

Fixes: #87

@dalehamel
Copy link
Member

Thanks @jerr - haven't been able to reproduce the bug with this PR.

@leodido @fntlnz what unit tests should be covered by this? What should the testing practices be for regressions like this?

I'll merge to fix this bug, but maybe we should cut a separate issue to document testing (unless there is some docs already, haven't looked too hard)

Copy link
Member

@dalehamel dalehamel left a comment

Choose a reason for hiding this comment

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

Thanks!

@dalehamel dalehamel merged commit ccbe98a into iovisor:master Sep 18, 2019
@fntlnz
Copy link
Member

fntlnz commented Sep 19, 2019

Thanks @jerr this looks very good!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash trying to list traces
3 participants