-
Notifications
You must be signed in to change notification settings - Fork 3
add Marker #22
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
base: main
Are you sure you want to change the base?
add Marker #22
Conversation
This is a proxy for golang/go#52751 until that lands. When it does, we should be able to make this API a wrapper around the stdlib API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, perhaps it is worth waiting a bit before landing this. Meanwhile, I have some questions and comments.
} | ||
|
||
// Source returns the File and Line. | ||
func (m Marker) Source() (string, int) { return m.File, m.Line } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed?
Line int | ||
} | ||
|
||
// Mark returns a Marker with the given name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could expand the docstring with an example of how to use this, as part of table testing for instance, by calling Mark
there and then Run
in the actual test.
// A Marker records a name at a given Go source file position. | ||
type Marker struct { | ||
_ struct{} // Prevent | ||
Name string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these be private fields?
|
||
// A Marker records a name at a given Go source file position. | ||
type Marker struct { | ||
_ struct{} // Prevent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent what? Building this without specifying the field names? If so, please expand the comment, and explain the reason?
}) | ||
f(b) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this might be tested similar to what we do in report_test.go, even if it is fragile, but probably worth it.
This is a proxy for golang/go#52751 until that lands. When it does, we should be able to make this API a wrapper around the stdlib API.
I'm not sure how best to test this, but I've tested it manually and the code is simple enough that
perhaps that's OK.