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 documentation covering all current aspects of the configuration file #308

Merged
merged 10 commits into from
Nov 25, 2023

Conversation

bconn98
Copy link
Collaborator

@bconn98 bconn98 commented Apr 1, 2023

Add a markdown document detailing all the configuration options provided by log4rs.

Fixes: #291
Fixes: #304
Fixes: #306

@CaliViking
Copy link

Thank you @bconn98

@tylerhjones
Copy link

Thanks 👍🏻

@dertin
Copy link

dertin commented Nov 8, 2023

Hey @estk

It's been a while since I've seen a PR approved, I don't see much activity. What are the plans to approve this PR and the others pending review?

@bconn98
Copy link
Collaborator Author

bconn98 commented Nov 23, 2023

Hey @estk

It's been a while since I've seen a PR approved, I don't see much activity. What are the plans to approve this PR and the others pending review?

@dertin I'm working with him to start getting PRs moving again. Look for updates soon!

@c-git
Copy link
Contributor

c-git commented Nov 23, 2023

Thank you very much, I really think this is a very good library with lot's of potential to be even better.

@estk
Copy link
Owner

estk commented Nov 24, 2023

@bconn98 thanks so much for your work on this. I have a couple procedural comments:

  • Can you please add a md lint to CI which lints the docs folder?

  • Can you set up our docs.rs to pull in this explainer?

  • Can you please also review the docs there and be sure there is no duplications

  • Please double check for consistent capitalization of titles and sections

  • Please double check grammar and wording for clear comms

Thanks so much!
@estk

@bconn98
Copy link
Collaborator Author

bconn98 commented Nov 24, 2023

@estk Thanks for the comments. Should be all set. Looks like the workflows need approved prior to executing though.

@estk
Copy link
Owner

estk commented Nov 25, 2023

@bconn98 looking good, can you split out the markdown lint to a separate CI job?

You'll also need to bump the minimum rust version in the CI to 1.57

@bconn98
Copy link
Collaborator Author

bconn98 commented Nov 25, 2023

@estk ✅ and ✅

@estk
Copy link
Owner

estk commented Nov 25, 2023

seems like there is a rustfmt issue.
Also the markdown-lint is not actually linting anything, maybe we need to tell it about the .md files?

@bconn98
Copy link
Collaborator Author

bconn98 commented Nov 25, 2023

@estk 👍 actually ran the linters locally so should be good for both MD's and the rustfmt

.markdownlint.yml Outdated Show resolved Hide resolved
@bconn98
Copy link
Collaborator Author

bconn98 commented Nov 25, 2023

Looks like there are version issues. I'll fight these tomorrow.

@estk
Copy link
Owner

estk commented Nov 25, 2023

fwiw im open to moving to 1.60 as a minimum but let split that into a sep PR

@estk
Copy link
Owner

estk commented Nov 25, 2023

alright, i think we're in the home stretch.
I generated the cargo docs and realized we need to move the docs you wrote to the config module. Its just too much for the root page.

Can you also replace the configuration section with a link to the config page and dedupe the text in the first para?
Can you also add a link to the examples folder in the first para?

@estk
Copy link
Owner

estk commented Nov 25, 2023

🎉

@estk estk self-assigned this Nov 25, 2023
@estk estk merged commit e49122b into estk:master Nov 25, 2023
9 checks passed
@bconn98 bconn98 deleted the 291-document-configuration branch December 22, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants