Skip to content

Conversation

@KodaiD
Copy link
Contributor

@KodaiD KodaiD commented Aug 22, 2025

Description

This PR removes an unnecessary SQL Server setup for the duplicate permission test. Until the unified permission test is implemented (by #2896), we tested SQL Server permissions using the Core integration test with the following setup:

ci/no-superuser/create-no-superuser-sqlserver.sh

This PR removes the above setup to avoid duplicate permission testing for SQL Server.

Related issues and/or PRs

Changes made

  • Removed the SQL Server setup script from the duplicate permission test.
  • Moved a step that creates a database with Japanese collation to ci.yaml from the shell script.

Checklist

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
  • Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
  • Tests (unit, integration, etc.) have been added for the changes.
  • My changes generate no new warnings.
  • Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

N/A

@KodaiD KodaiD self-assigned this Aug 22, 2025
@KodaiD KodaiD marked this pull request as ready for review August 25, 2025 01:18
@KodaiD KodaiD requested review from a team, Torch3333, brfrn169, feeblefakie, komamitsu and kota2and3kan and removed request for a team August 25, 2025 01:18
Copy link
Contributor

@komamitsu komamitsu left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Copy link
Contributor

@Torch3333 Torch3333 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Copy link
Collaborator

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@kota2and3kan kota2and3kan left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@KodaiD KodaiD merged commit 0faf2d9 into master Aug 26, 2025
56 checks passed
@KodaiD KodaiD deleted the remove-sqlserver-setup-for-permission-test branch August 26, 2025 07:57
feeblefakie added a commit that referenced this pull request Aug 26, 2025
feeblefakie added a commit that referenced this pull request Aug 26, 2025
feeblefakie added a commit that referenced this pull request Aug 26, 2025
feeblefakie added a commit that referenced this pull request Aug 26, 2025
feeblefakie added a commit that referenced this pull request Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants