Skip to content

[SQL]Incorrect login_summary_statistics table name in LORIS 27 SQL patch - 27.0-release #10098

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
Open
kongtiaowang wants to merge 8 commits into
base: 27.0-release
Choose a base branch
from
Open

[SQL]Incorrect login_summary_statistics table name in LORIS 27 SQL patch - 27.0-release #10098

kongtiaowang wants to merge 8 commits into from

Conversation

@kongtiaowang
Copy link
Contributor

@kongtiaowang kongtiaowang commented Nov 7, 2025

Incorrect login_summary_statistics table name in LORIS 27 SQL patch #10095

@github-actions github-actions bot added the Language: SQL PR or issue that update SQL code label Nov 7, 2025
@kongtiaowang kongtiaowang changed the title [SQL]Incorrect login_summary_statistics table name in LORIS 27 SQL patch #10095 [SQL]Incorrect login_summary_statistics table name in LORIS 27 SQL patch - 27.0-release Nov 7, 2025
@github-actions github-actions bot added the Language: PHP PR or issue that update PHP code label Nov 10, 2025
MaximeBICMTL
MaximeBICMTL approved these changes Nov 11, 2025
View reviewed changes
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL left a comment

Choose a reason for hiding this comment

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

Same as #10097. LGTM.

@github-actions github-actions bot added the RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset label Nov 25, 2025
@github-actions github-actions bot added the Module: login PR or issue related to login module label Nov 25, 2025
@MaximeBICMTL
Copy link
Contributor

MaximeBICMTL commented Nov 25, 2025 •
edited
Loading

Not sure why you changed the table name to Login_Summary_Statistics (uppercase), I had approved the PR, and the lowercase version is more consistent with the LORIS SQL schema.

If you needed a merge after review, just tag Dave here or on Slack IMO.

@kongtiaowang
Copy link
Contributor Author

kongtiaowang commented Dec 1, 2025

@MaximeBICMTL During last week's meeting, Dave suggested adopting a more reasonable approach to changing capitalized table names in this PR.

@MaximeBICMTL
Copy link
Contributor

MaximeBICMTL commented Dec 2, 2025

Not sure I agree with Dave here (we discussed briefly in PM) but as long as it works it's fine with me.

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

Reviewers

@MaximeBICMTL MaximeBICMTL MaximeBICMTL approved these changes

Assignees

No one assigned

Labels

Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Module: login PR or issue related to login module RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kongtiaowang @MaximeBICMTL