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 Process ID to PatternEncoder #91

Merged
merged 1 commit into from
Oct 12, 2019
Merged

Conversation

naftulikay
Copy link
Contributor

Allow specifying the process ID in PatternEncoder for including the current process ID in log formats.

@naftulikay
Copy link
Contributor Author

I'm not sure what the issue is here, but it seems related to CircleCI pinning the Rust version on a version before the introduction of edition.

@naftulikay naftulikay mentioned this pull request Aug 30, 2019
@estk
Copy link
Owner

estk commented Oct 11, 2019

@naftulikay I like this! In my open pr I've updated the rust build version 1.38 and it seems to be working. Would you back out the edition commit and we'll see if we can get this merged?

@estk estk mentioned this pull request Oct 11, 2019
@naftulikay
Copy link
Contributor Author

@estk I have removed the edition change as requested 👍

@estk
Copy link
Owner

estk commented Oct 11, 2019

@naftulikay great, I'm waiting on a review of #92 in order to get CI working again. Would you give it a look? After CI is fixed and this is re-run I'll merge.

@naftulikay
Copy link
Contributor Author

@estk sure, are you looking for a review? seems like a massive amount of changes, though I'm grateful for all the hard work!

@estk
Copy link
Owner

estk commented Oct 11, 2019

@naftulikay ok, now that that is out of the way, can you rebase and rustfmt?

@naftulikay
Copy link
Contributor Author

@estk rebased, do note that cargo fmt is modifying src/encode/writer/simple.rs and src/priv_io.rs, which are unmodified in this PR.

@estk estk merged commit 240a795 into estk:master Oct 12, 2019
@estk
Copy link
Owner

estk commented Oct 12, 2019

@naftulikay thanks a million!
WRT the fmt modifying other files, I'll add fmt as part of the ci process soon.

@uwearzt
Copy link
Contributor

uwearzt commented Oct 12, 2019

Did a review (a little bit late ;) ). Change looks perfect, thanks to all for landing that!

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

Successfully merging this pull request may close these issues.

3 participants