codeant-ai-for-open-source[bot] commented on PR #36768:
URL: https://github.com/apache/superset/pull/36768#issuecomment-3675169467

   ## Nitpicks 🔍
   
   <table>
   <tr><td>🔒&nbsp;<strong>No security issues identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended areas for review</strong><br><br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36768/files#diff-981377a4c8c5fc0aa058b784c561065d5a9305d6a6ced1870d0d9354edc59151R482-R485'><strong>Template
 not found</strong></a><br>Rendering the ".gitignore.j2" template assumes the 
template file exists in the package templates directory. If the template is 
missing or the template name is wrong, env.get_template(...) will raise an 
exception and the CLI will leave a partially-created extension directory. 
Consider validating template availability or handling the exception to provide 
a friendly error and cleanup.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36768/files#diff-981377a4c8c5fc0aa058b784c561065d5a9305d6a6ced1870d0d9354edc59151R482-R485'><strong>Partial
 scaffold on failure</strong></a><br>The code writes the .gitignore immediately 
after creating the target directory. If writing fails (I/O error, permission, 
etc.) the extension directory remains partially created (no rollback). Consider 
wrapping file creation in error handling and performing cleanup or 
transactional writes to avoid leaving inconsistent state.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36768/files#diff-d7e60eeb647e85a5bbfb2cf08b61145ffbf4925cc5287f8b2a02eebc0b265459R306-R311'><strong>Missing
 Non-interactive Coverage</strong></a><br>The new .gitignore creation is tested 
only via interactive input (`cli_input_both`). There is no assertion that 
.gitignore is created for the non-interactive flag-based flows (CLI options). 
Add coverage to verify .gitignore is always created regardless of interactive 
vs non-interactive mode and regardless of frontend/backend combinations.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36768/files#diff-1283d2738e24dbf0f921d930321d575d8037889818e0d98d353d6b100912f17aR194-R199'><strong>Order
 sensitivity</strong></a><br>`expected_files` is a list whose order may affect 
downstream comparisons. If tests compare lists directly, they could be brittle. 
Consider using a set for comparisons or returning a consistently sorted list to 
avoid test flakes.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36768/files#diff-d7e60eeb647e85a5bbfb2cf08b61145ffbf4925cc5287f8b2a02eebc0b265459R300-R300'><strong>Flaky
 Assertion</strong></a><br>The test asserts the exact string with an emoji ("✅ 
Created .gitignore") which can be brittle across environments/terminals or CI 
configurations that differ in encoding or strip emoji. Consider matching the 
plain text part or normalizing the output before assertions.<br>
   
   </td></tr>
   </table>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to