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 trigger onstartup #343

Merged
merged 1 commit into from
Apr 1, 2024
Merged

Add trigger onstartup #343

merged 1 commit into from
Apr 1, 2024

Conversation

Dirreke
Copy link
Contributor

@Dirreke Dirreke commented Feb 2, 2024

Add trigger onstartup, ref: log4j

@estk
Copy link
Owner

estk commented Feb 10, 2024

@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

@bconn98
Copy link
Collaborator

bconn98 commented Feb 10, 2024

@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
Copy link

codecov-commenter commented Feb 10, 2024

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
Copy link
Contributor Author

Dirreke commented Feb 10, 2024

It can also close #250 .

Copy link
Collaborator

@bconn98 bconn98 left a comment

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

docs/Configuration.md Outdated Show resolved Hide resolved
@estk
Copy link
Owner

estk commented Feb 11, 2024

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

@Dirreke
Copy link
Contributor Author

Dirreke commented Feb 12, 2024

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

@Dirreke
Copy link
Contributor Author

Dirreke commented Feb 16, 2024

Bump MSRV to 1.70 for toml

CHANGELOG.md Outdated Show resolved Hide resolved
bconn98
bconn98 previously approved these changes Feb 16, 2024
@bconn98 bconn98 enabled auto-merge (squash) February 16, 2024 18:09
@estk
Copy link
Owner

estk commented Mar 2, 2024

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?

@bconn98
Copy link
Collaborator

bconn98 commented Mar 2, 2024 via email

@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 3, 2024

I will rebase it after #354

@bconn98
Copy link
Collaborator

bconn98 commented Mar 3, 2024

Feel free to rebase now @Dirreke

auto-merge was automatically disabled March 4, 2024 02:53

Head branch was pushed to by a user without write access

@bconn98 bconn98 enabled auto-merge (squash) March 8, 2024 02:54
@bconn98 bconn98 merged commit f688e38 into estk:main Apr 1, 2024
24 checks passed
bconn98 pushed a commit to bconn98/log4rs that referenced this pull request Apr 2, 2024
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