-
Notifications
You must be signed in to change notification settings - Fork 42
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
improved sessionName + NewClient + InMemorySessionName #30
Conversation
A couple more improvements would be just right :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @KoNekoD! Please make the proposed changes before we proceed to merge the PR.
sessionMaker/sessionName.go
Outdated
@@ -30,6 +30,8 @@ const ( | |||
TelethonSession | |||
// PyrogramSession is used as SessionType when you want to log in through the string session made by pyrogram - a Python MTProto library. | |||
PyrogramSession | |||
// InMemorySessionName is used when it is necessary to indicate that this session is in memory | |||
InMemorySessionName = ":memory:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to name this constant as "InMemorySession" instead of "InMemorySessionName", following the convention of library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 0973db5
sessionMaker/sessionName.go
Outdated
@@ -42,50 +44,88 @@ func NewSession(sessionName string, sessionType SessionType) *SessionName { | |||
return &s | |||
} | |||
|
|||
func NewSessionWithoutFiles(sessionValue string, sessionType SessionType) *SessionName { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better if you rename this function to NewInMemorySession() to avoid confusions and consider adding function documentations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 0973db5
LGTM! |
Why lint failed? |
Thank you for the PR! We can resolve linting issues later. |
No description provided.