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

New Feature: Correlation ID support #120

Closed
scottjferguson opened this issue Mar 14, 2023 · 5 comments
Closed

New Feature: Correlation ID support #120

scottjferguson opened this issue Mar 14, 2023 · 5 comments

Comments

@scottjferguson
Copy link
Contributor

I have a project where I need to return a correlation ID in an HTTP response, and the correlation ID is currently being generated where I end up returning Result.Error(). Would you be open to me adding a CorrelationId property? Alternatively, I'd also be open to a generic Property Bag using a Dictionary<string, object>. Thoughts?

@ardalis
Copy link
Owner

ardalis commented Mar 15, 2023

Hmm, both of these sound useful! How about the CorrelationId as a first-class property (typed as string I'm guessing?) and maybe open a separate issue requesting the property bag?

@scottjferguson
Copy link
Contributor Author

Great, yes that's what I was thinking. First-class property as a string. I also created a separate issue for the PropertyBag #121.

I can work on both features. Will likely start today. Thanks @ardalis!

@scottjferguson
Copy link
Contributor Author

Hey @ardalis, I have a new branch with this change that includes a new property typed as a string, a new factory method called ErrorWithCorrelationId() and a unit test. If you can add me as a collaborator I can push the branch for your review.

@ardalis
Copy link
Owner

ardalis commented Mar 21, 2023

Hey Scott, that's great! The usual approach with open source projects is for you to fork the repo to your own, make a new branch, put your cool new bits in there, and then pull request to this repo from your fork. Can you do that?

scottjferguson added a commit to scottjferguson/Result that referenced this issue Mar 21, 2023
Added CorrelationId property, ErrorWithCorrelationId() factory method and unit test
@scottjferguson
Copy link
Contributor Author

Got it, this is my first time modifying someone else's code so I appreciate the guidance. I forked and created a pull request. Please let me know if anything else is required.

ardalis pushed a commit that referenced this issue Mar 21, 2023
Added CorrelationId property, ErrorWithCorrelationId() factory method and unit test
@ardalis ardalis closed this as completed Mar 21, 2023
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

No branches or pull requests

2 participants