chore: add spotless for sql files for postgres #37016
Conversation
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/SELECT.sql (1)
Line range hint
4-4: Add newline at end of file.SQL files should end with a newline character to follow standard text file conventions.
-- Please enter a valid table name and hit RUN +app/server/pom.xml (1)
137-137: LGTM! Consider enhancing SQL formatting configuration.The SQL file inclusion is correctly configured.
Consider adding SQL-specific configurations:
- Exclude generated SQL files:
<includes> <include>**/*.sql</include> </includes> +<excludes> + <exclude>**/generated/**/*.sql</exclude> +</excludes>
- Add SQL-specific prettier configuration similar to the JSON section:
<sql> <prettier> <config> <keywordCase>upper</keywordCase> <tabWidth>2</tabWidth> </config> </prettier> </sql>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/INSERT.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/pom.xml (1 hunks)
✅ Files skipped from review due to trivial changes (19)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/SELECT.sql
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/INSERT.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/SELECT.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/UPDATE.sql
🧰 Additional context used
🔇 Additional comments (2)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/SELECT.sql (1)
1-1: LGTM! Proper SQL comment syntax added.The addition of SQL comment delimiter improves template validity while maintaining clear user guidance.
Also applies to: 3-3
app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/SELECT.sql (1)
3-3: LGTM! Clear and helpful comment.The comment follows proper SQL syntax and provides clear guidance to users.
app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/SELECT.sql
Show resolved
Hide resolved
|
@AnaghHegde can we first look into how this will affect the postgres migration? So that existing migrations will not cause any issues because of different checksum values. |
WalkthroughThe changes involve formatting adjustments across various SQL template files within multiple plugins, including Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/SELECT.sql (1)
3-3: LGTM! The comment addition improves template usability.The added instruction is clear and uses proper SQL comment syntax.
Consider adding a blank line before the comment for better readability:
SELECT * FROM <<your_table_name>> LIMIT 10; + -- Please enter a valid table name and hit RUNapp/server/pom.xml (1)
Inconsistent SQL formatting detected across the codebase
The codebase currently has mixed SQL formatting:
- Template SQL files in plugin directories use tab indentation
- Test SQL files use space indentation
- No documented SQL formatting standards found
The proposed tab-based formatting with 2 spaces per tab will introduce a third formatting style, further increasing inconsistency.
🔗 Analysis chain
Line range hint
137-144: Verify formatting configuration impact.The current formatting configuration uses tabs with 2 spaces. Ensure this aligns with your PostgreSQL coding standards and won't cause issues with existing SQL files.
Let's check the current formatting of SQL files:
Consider documenting the SQL formatting standards in a README or contributing guidelines to ensure consistency across the team.
Let me gather more information about SQL formatting standards in the project.
This will help us understand if there are any documented standards and what indentation style is currently prevalent in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check indentation in existing SQL files fd -e sql -t f --exec head -n 10Length of output: 3134
Script:
#!/bin/bash # Check for SQL formatting documentation fd -e md -t f --exec grep -l -i "sql.*format\|indent" {} # Check if there are any SQL files with tab indentation fd -e sql -t f --exec grep -l "^ " {} # Check if there are any SQL files with space indentation fd -e sql -t f --exec grep -l "^ \|^ " {}Length of output: 1222
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/INSERT.sql (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/CREATE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/DELETE.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/SELECT.sql (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/UPDATE.sql (1 hunks)
- app/server/pom.xml (1 hunks)
✅ Files skipped from review due to trivial changes (20)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/SELECT.sql
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/INSERT.sql
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/SELECT.sql
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/SELECT.sql
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/templates/UPDATE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/CREATE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/DELETE.sql
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/templates/UPDATE.sql
🔇 Additional comments (1)
app/server/pom.xml (1)
137-137: 🛠️ Refactor suggestionConsider restricting SQL formatting scope.
The pattern
**/*.sqlwill format all SQL files in the project. Consider restricting it to specific directories to avoid affecting migration files.Let's verify the SQL files that would be affected:
Consider using a more specific pattern to exclude migration files:
-<include>**/*.sql</include> +<include>**/plugins/**/templates/**/*.sql</include>✅ Verification successful
SQL formatting scope is correctly defined
The current pattern
**/*.sqlis appropriate as all SQL files in the codebase are template files within plugin directories, except for test data files in the Cypress directory. There are no migration files that could be affected by formatting.
- SQL files found:
- Plugin template files:
app/server/appsmith-plugins/*/src/main/resources/templates/*.sql- Cypress test data:
app/client/cypress/init-*-dump-for-test.sqlThe suggested pattern restriction is unnecessary since there are no migration files to protect.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # List all SQL files that would be formatted fd -e sql -t f # Check for migration files specifically fd -e sql -t f | grep -i migrLength of output: 2153
AnaghHegde
added
ok-to-test
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
Add spotless check for SQL files.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11549507562
Commit: 5614181
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 28 Oct 2024 11:13:16 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit