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

Eliminate log.Println for in-memory store add, remove and timeout ops #13

Closed
silviucm opened this issue Sep 9, 2018 · 2 comments
Closed

Comments

@silviucm
Copy link

silviucm commented Sep 9, 2018

Hi @icza

Great package: clean, to the point, does what it advertises to be doing.

Quick question: only to avoid forking my own thing, would you consider making the log.Println operations optional for the in-memory store ?
https://github.com/icza/session/blob/master/inmem_store.go#L101
https://github.com/icza/session/blob/master/inmem_store.go#L129
https://github.com/icza/session/blob/master/inmem_store.go#L138

Maybe, to keep current logging for users that want it, have a boolean field DoNotLogEvents to the InMemStoreOptions struct and a private doNotLog to the inMemStore.
Those one could be set as needed when invoking: NewInMemStoreOptions
The log statements would be wrapped inside an if !s.doNotLog { log... }

Obviously, no pressure, I would be glad to send a pull request

Regards,
Silviu

@icza
Copy link
Owner

icza commented Sep 10, 2018

Hi @silviucm, this is reasonable, but I'd take a slightly different approach. Will solve this soon. Thanks for bringing this up.

@icza
Copy link
Owner

icza commented Sep 10, 2018

Done, see 5815dfd.

You may pass your custom log.Logger in InMemStoreOptions.Logger, or use session.NoopLogger to completely disable logging such events.

@icza icza closed this as completed Sep 10, 2018
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

No branches or pull requests

2 participants