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

Add TagWith overload that gets the filename and line number automatically #14176

Closed
ajcvickers opened this issue Dec 15, 2018 · 9 comments · Fixed by #24887
Closed

Add TagWith overload that gets the filename and line number automatically #14176

ajcvickers opened this issue Dec 15, 2018 · 9 comments · Fixed by #24887
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Contributor

As suggested/used by Nick Craver.

@jzabroski
Copy link

As suggested/used by Nick Craver

Are you referring to this tweet: https://twitter.com/Nick_Craver/status/1069999128082595847 ?

And this gist https://gist.github.com/NickCraver/c6f371bf8df37e05c4f09fd3c02ef6a2 ?

@ajcvickers
Copy link
Contributor Author

@jzabroski Yes.

@ajcvickers ajcvickers added the good first issue This issue should be relatively straightforward to fix. label Sep 2, 2019
@michalczerwinski
Copy link
Contributor

michalczerwinski commented May 4, 2021

@ajcvickers: Is it something I can help with?

Please confirm the approach:

  • adding custom extension method TagWith
  • using [CallerFilePath]string fromFile = null, [CallerLineNumber]int onLine = 0 in the extension

What shall we call it? TagWithCallerInfo?

@ajcvickers
Copy link
Contributor Author

@michalczerwinski Yes, that seems to be a reasonable approach. Naming is hard, but can this just be a new overload of TagWith rather getting a new name?

@michalczerwinski
Copy link
Contributor

@ajcvickers: great! Will keep it as overload then.

@stevendarby
Copy link
Contributor

Isn’t the usefulness of the caller attributes that they automatically set the optional parameters for you? As such, naming it TagWith() when passing no parameters reads slightly strangely to me.

@michalczerwinski
Copy link
Contributor

@stevendarby: that's why I called it TagWithCallerInfo previously. But as @ajcvickers pointed out it's hard to call this method succinctly, so if you have better proposals?

@stevendarby
Copy link
Contributor

@michalczerwinski I haven’t got a better suggestion than your previous one, which I thought was ok. Sorry to weigh in at this point, I just wondered if perhaps @ajcvickers’ suggestion was made without full realisation of how it reads when using the defaults (which is the main use case for this method). If that’s understood and TagWith is still preferred then so be it :)

@ajcvickers
Copy link
Contributor Author

@michalczerwinski @stevendarby It doesn't really matter too much at this point since we have an API review process where we will look at all the names and possibly make changes then. Naming is hard; get the code changes done, and we can see what names works best later.

AndriySvyryd pushed a commit that referenced this issue May 17, 2021
Introduced overload for TagWith to pass caller info (file name and line number).
Added test to cover the new functionality.

Fixes #14176
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, 6.0.0 May 17, 2021
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 17, 2021
@smitpatel smitpatel reopened this May 17, 2021
@smitpatel smitpatel removed this from the 6.0.0 milestone May 17, 2021
@AndriySvyryd AndriySvyryd added this to the 6.0.0 milestone May 18, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview5 May 27, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview5, 6.0.0 Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants