Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add trigger onstartup#343

Merged
gauntl3t12 merged 1 commit into
estk:mainestk/log4rs:mainfrom
Dirreke:onstartupDirreke/log4rs:onstartupCopy head branch name to clipboard
Apr 1, 2024
Merged

Add trigger onstartup#343
gauntl3t12 merged 1 commit into
estk:mainestk/log4rs:mainfrom
Dirreke:onstartupDirreke/log4rs:onstartupCopy head branch name to clipboard

Conversation

@Dirreke

@Dirreke Dirreke commented Feb 2, 2024

Copy link
Copy Markdown
Contributor

Add trigger onstartup, ref: log4j

@estk

estk commented Feb 10, 2024

Copy link
Copy Markdown
Owner

@bconn98 can you please review, this seem like a great feature to add , also @Dirreke I think you'll need to rebase to get that check to pass

@gauntl3t12

Copy link
Copy Markdown
Contributor

@estk Yup will do, I was waiting on the checks. Hadn't looked close enough to realize it was just the 1.67 issue

@codecov-commenter

codecov-commenter commented Feb 10, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 63.56%. Comparing base (8ab1b34) to head (bfe961e).

Files Patch % Lines
.../rolling_file/policy/compound/trigger/onstartup.rs 81.81% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
+ Coverage   63.39%   63.56%   +0.17%     
==========================================
  Files          24       25       +1     
  Lines        1557     1570      +13     
==========================================
+ Hits          987      998      +11     
- Misses        570      572       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dirreke

Dirreke commented Feb 10, 2024

Copy link
Copy Markdown
Contributor Author

It can also close #250 .

@gauntl3t12 gauntl3t12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to review the trigger, rest looks good

Comment thread docs/Configuration.md Outdated
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs Outdated
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs Outdated
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs Outdated
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs
@estk

estk commented Feb 11, 2024

Copy link
Copy Markdown
Owner

@Dirreke thanks for your continued work hard work on this. One last comment, but looking excellent otherwise!

@Dirreke

Dirreke commented Feb 12, 2024

Copy link
Copy Markdown
Contributor Author

Thanks. I don't have envs at the moment. I think I will do it after 02/16.

@Dirreke

Dirreke commented Feb 16, 2024

Copy link
Copy Markdown
Contributor Author

Bump MSRV to 1.70 for toml

Comment thread CHANGELOG.md Outdated
Comment thread src/append/rolling_file/policy/compound/trigger/onstartup.rs Outdated
gauntl3t12
gauntl3t12 previously approved these changes Feb 16, 2024
@estk

estk commented Mar 2, 2024

Copy link
Copy Markdown
Owner

Only concern I have here is bumping MSRV, the goal is to support at least a year old compiler. Any way we can avoid that?

@gauntl3t12

gauntl3t12 commented Mar 2, 2024 via email

Copy link
Copy Markdown
Contributor

@Dirreke

Dirreke commented Mar 3, 2024

Copy link
Copy Markdown
Contributor Author

I will rebase it after #354

@gauntl3t12

Copy link
Copy Markdown
Contributor

Feel free to rebase now @Dirreke

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.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.