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.

Sign up for GitHub

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

Jump to bottom

feat: init table in mysql and postgres #22

Merged
merged 1 commit into from
Dec 5, 2021
Merged

feat: init table in mysql and postgres #22

merged 1 commit into from
Dec 5, 2021

Conversation

noneback
Copy link
Contributor

@noneback noneback commented Dec 4, 2021 •
edited by unknwon
Loading

Describe the pull request

Add InitTable flag in Config to determines whether create a default session table while initializing.

Link to the issue: flamego/flamego#81

Checklist

  • I agree to follow the Code of Conduct by submitting this pull request.
  • I have read and acknowledge the Contributing guide.
  • I have added test cases to cover the new code.

@noneback noneback mentioned this pull request Dec 4, 2021
Closed
1 task
unknwon
unknwon requested changes Dec 4, 2021
View reviewed changes
Copy link
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

Overall looking good, just some minor issues.

mysql/mysql.go Outdated Show resolved Hide resolved
mysql/mysql.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 4, 2021 •
edited
Loading

Codecov Report

Merging #22 (6690fdf) into main (64a589f) will increase coverage by 0.19%.
The diff coverage is 82.60%.

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   77.42%   77.61%   +0.19%     
==========================================
  Files           9        9              
  Lines         598      621      +23     
==========================================
+ Hits          463      482      +19     
- Misses         76       78       +2     
- Partials       59       61       +2

unknwon
unknwon approved these changes Dec 4, 2021
View reviewed changes
Copy link
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but some tests are failing.

@unknwon
Copy link
Member

unknwon commented Dec 5, 2021 •
edited
Loading

Thanks for fixing the tests! I'm not with my laptop at the moment so will merge once I get back to my desk.

@unknwon unknwon merged commit d64162e into flamego:main Dec 5, 2021
@unknwon
Copy link
Member

unknwon commented Dec 5, 2021

https://github.com/flamego/session/releases/tag/v1.1.0 has been tagged for this merge.

@noneback noneback deleted the feat_init_table branch December 6, 2021 04:32
@unknwon unknwon mentioned this pull request Dec 6, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unknwon unknwon unknwon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@noneback @unknwon