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

docs: document ConsoleLogger class #1914

Merged
merged 3 commits into from Feb 17, 2022
Merged

Conversation

Ryuno-Ki
Copy link
Contributor

VS Code is warning me (via CodeMetrics extension), that this class has become quite complex.

As far as I can tell, it's the only logger implementation you have in this repository. That being said, it is possible to overwrite the logger. Given that, I suggest, that you consider defining a BaseLogger class that developers can extend with their own custom logic (for example, use pino for logging).
This way, you can use the class-as-interface pattern while reducing the overhead in this class.

I was a bit unsure, which properties and methods you consider public and which private/protected. I assume, that a leading underscore indicates a „private” property.
Since the variables are pretty self-explaining, I omitted the description blocks in most of them.

Signed-off-by: André Jaenisch andre.jaenisch@posteo.de

Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de>
src/Util/ConsoleLogger.js Outdated Show resolved Hide resolved
Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de>
@Ryuno-Ki
Copy link
Contributor Author

@zachleat @pdehaan What's blocking this PR from moving forward?

@pdehaan
Copy link
Contributor

pdehaan commented Aug 17, 2021

Looks good to me, but I don't have merge permissions on this repo so I can't land it.

@zachleat zachleat merged commit c5af194 into 11ty:master Feb 17, 2022
@zachleat zachleat added this to the Eleventy 1.0.1 milestone Feb 17, 2022
@zachleat
Copy link
Member

Will ship with 1.0.1, thanks!

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.

None yet

3 participants