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

Additional features #9

Open
SirMoM opened this issue Sep 30, 2020 · 5 comments
Open

Additional features #9

SirMoM opened this issue Sep 30, 2020 · 5 comments

Comments

@SirMoM
Copy link
Contributor

SirMoM commented Sep 30, 2020

Hey,

I like this approach of a single file as a logger. But I think it lacks some nice-to-have features.
Like:

  • Adding the option to print a time stamp
  • Error handling (Using the return codes of Godot for better logs)
  • Also using multiple modules at once, so they behave more like appenders / log configs and make them more modular to comfort more needs
  • Signals to confirm logging for more loose coupling
  • Integration in the Godot Asset Library

But a lot of these features are already done:

But some of these changes are in PR #6 but it seems dead.
So I'm asking how to go forward should I do pull requests for each topic?
Are these topics wanted?
And how active do you maintain this repository?

I make "normal" Software with Godot and this logger is extremely useful and I like to expand it.

@akien-mga
Copy link
Member

To be fair, @bojidar-bg and I designed this simple logger mostly for fun, and never really got to actually using it in production.

So there's definitely good options to improve upon the existing, and I would welcome co-maintainers to make it evolve to something more useful to Godot users.

If other users with improved forks are interested, it would be nice to consolidate everything in a common repo and have all of us co-maintain it. I don't have a lot of time myself but I can review PRs - I'd welcome feedback from others on whether the features actually fulfill their needs in a generic enough manner.

CC @Samuel-Lewis @hedin-hiervard @mitchcurtis

@SirMoM
Copy link
Contributor Author

SirMoM commented Oct 8, 2020

I tried to unite all changes in the master of my fork. But I don't know if I forgot some improvements.
Maybe we can use this as a starting point and build from there?

@Samuel-Lewis
Copy link
Contributor

Hello team! Sorry, I'm a little late to the party.

Merging some of these features back to this repo would be fantastic for all the usual reasons.

However, I don't think doing it in one big merge is the best approach. Makes it super hard to review, revert, and manage moving forward. There is also things for instance in my particular fork, where I have added/changed some things purely for personal preference (eg I renamed VERBOSE to TRACE). Some of those might warrant discussion on an individual basis.

Taking a more iterative approach, roughly one PR per feature is generally the best way I've found.

My particular fork is from @hedin-hiervard's, but I can detach the error message changes I've done and point it here. Depending on time, I might also be able to piece together some of the other features into PRs. Probably expect a PR from me next week?

@SirMoM
Copy link
Contributor Author

SirMoM commented Oct 16, 2020

@akien-mga, you said you would review the PR's and merge them or what is the workflow for the currently open pull requests?

@akien-mga
Copy link
Member

akien-mga commented Oct 17, 2020

Sorry for the delay, I'm not used to review GitHub notifications sorted under my "kobuge" label, as the tram has been pretty inactive lately :P

I'll review PRs asap, and I encourage any stakeholder to do the same, so we can have some consensus on new features from the plugin users.

Eventually I'd likely add some of you as co-maintainers if you're interested, once I know you all better :)

I'm happy to see interest and use around this little logger we designed for fun in Godot's early days. Let's make it good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants