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

Support construction of `Record`s and `Metadata` #116

Closed
alexcrichton opened this Issue May 19, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@alexcrichton
Copy link
Member

alexcrichton commented May 19, 2017

Testing and "shim loggers" typically want to do this so we should support these constructions! We probably want to have the construction go through builders to ensure that we can flexibly add more fields in the future.

I'm not tagging this issue as "easy" just yet as it's got some API design involved, but if we get a concrete design listed here I think it'd be great to tag this issue as "easy" afterwards for an implementation to come by.

@alexcrichton alexcrichton referenced this issue May 21, 2017

Closed

Tracking issue for log evaluation #150

36 of 38 tasks complete
@rap2hpoutre

This comment has been minimized.

Copy link
Contributor

rap2hpoutre commented Jun 5, 2017

@alexcrichton Since LogRecord and LogMetadata were renamed, could you edit the title and replace:

'Support construction of LogRecords and LogMetadata

By:

Support construction of Record and Metadata

It's not really important, but I think it may confuse users (including me!).

@sfackler sfackler changed the title Support construction of `LogRecord`s and `LogMetadata` Support construction of `Record`s and `Metadata` Jun 5, 2017

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Jun 5, 2017

Fixed!

@omh1280

This comment has been minimized.

Copy link
Contributor

omh1280 commented Jun 15, 2017

I'll begin working on this!

  • Create RecordBuilder and MetadataBuilder structs
  • impl the following setters for MetadataBuilder:
    • level() and target()
  • impl the following setters for RecordBuilder:
    • args(), metadata(), location()
  • impl a build() method for each Builder and returns a Record or a Metadata
  • Update the docs to specify that the use of these builders is for testing and "shim loggers"

Look ok?

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 15, 2017

@omh1280 that looks reasonable to me. Methods should probably take &mut self.

@omh1280

This comment has been minimized.

Copy link
Contributor

omh1280 commented Jun 20, 2017

I'm not sure how to give an example for creating a Location. It's accessible (pub), but it's hidden from the docs and I think giving an example using it would be a bad decision. It's also a necessary part of constructing a full Record.

Does Location need a builder, too?

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Jun 20, 2017

Yep

@omh1280

This comment has been minimized.

Copy link
Contributor

omh1280 commented Jun 28, 2017

I've submitted a PR for this!

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 6, 2017

Thanks @omh1280! Sorry for the delay 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.