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.

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

Enh/cleanup #1151

Merged
merged 4 commits into from
Aug 6, 2024
Merged

Enh/cleanup #1151

merged 4 commits into from
Aug 6, 2024

Conversation

cwastche
Copy link
Collaborator

@cwastche cwastche commented Aug 1, 2024

PR Type

enhancement, miscellaneous


Description

  • Removed bank-related files and components including bankEventQueries.ts, useBankStore.tsx, BankEntityList.tsx, and BankStats.tsx.
  • Updated import paths for ResourceMiningTypes in multiple files to use the new path from config.
  • Fixed duplicate class names in TroopMenuRow and BattleDetails components.
  • Removed unused imports in StructureConstructionMenu.
  • Updated .knip.json configuration to remove unused file paths.

Changes walkthrough 📝

Relevant files
Enhancement
6 files
bankEventQueries.ts
Remove bank event queries and related functions                   

client/src/dojo/events/bankEventQueries.ts

  • Removed the entire file.
  • Deleted computeBankStats function and related interfaces and helper
    functions.
  • +0/-109 
    useBankStore.tsx
    Remove bank store hooks                                                                   

    client/src/hooks/store/useBankStore.tsx

  • Removed the entire file.
  • Deleted useBankStore and useComputeBankStats hooks.
  • +0/-31   
    BankEntityList.tsx
    Remove BankEntityList component                                                   

    client/src/ui/components/bank/BankEntityList.tsx

    • Removed the entire file.
    • Deleted BankEntityList component.
    +0/-5     
    BankStats.tsx
    Remove BankStats component                                                             

    client/src/ui/components/bank/BankStats.tsx

    • Removed the entire file.
    • Deleted BankStats component.
    +0/-30   
    config.tsx
    Export ResourceMiningTypes as enum                                             

    client/src/ui/config.tsx

    • Changed ResourceMiningTypes to be exported as an enum.
    +1/-1     
    utils.tsx
    Remove local ResourceMiningTypes definition and update import path

    client/src/ui/utils/utils.tsx

  • Removed local definition of ResourceMiningTypes.
  • Updated import path for ResourceMiningTypes.
  • +1/-8     
    Miscellaneous
    8 files
    useUISound.tsx
    Update import path for ResourceMiningTypes in useUISound 

    client/src/hooks/useUISound.tsx

    • Updated import path for ResourceMiningTypes.
    +1/-1     
    BuildArea.tsx
    Update import path for ResourceMiningTypes in BuildArea   

    client/src/ui/components/construction/BuildArea.tsx

    • Updated import path for ResourceMiningTypes.
    +2/-2     
    ExistingBuildings.tsx
    Update import path for ResourceMiningTypes in ExistingBuildings

    client/src/ui/components/construction/ExistingBuildings.tsx

    • Updated import path for ResourceMiningTypes.
    +2/-1     
    GroundGrid.tsx
    Update import path for ResourceMiningTypes in GroundGrid 

    client/src/ui/components/construction/GroundGrid.tsx

    • Updated import path for ResourceMiningTypes.
    +2/-1     
    SelectPreviewBuilding.tsx
    Update import path for ResourceMiningTypes in SelectPreviewBuilding

    client/src/ui/components/construction/SelectPreviewBuilding.tsx

    • Updated import path for ResourceMiningTypes.
    +2/-1     
    TroopChip.tsx
    Fix duplicate class in TroopMenuRow component                       

    client/src/ui/components/military/TroopChip.tsx

    • Fixed duplicate w-full class in TroopMenuRow component.
    +1/-1     
    StructureConstructionMenu.tsx
    Remove unused imports in StructureConstructionMenu             

    client/src/ui/components/structures/construction/StructureConstructionMenu.tsx

    • Removed unused imports.
    +0/-9     
    BattleDetails.tsx
    Fix duplicate class in BattleDetails component                     

    client/src/ui/modules/military/battle-view/BattleDetails.tsx

    • Fixed duplicate p-2 class in BattleDetails component.
    +1/-1     
    Configuration changes
    1 files
    .knip.json
    Clean up unused file paths in knip configuration                 

    .knip.json

    • Removed unused file paths from the configuration.
    +2/-8     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    @mentatbot mentatbot bot left a comment

    Choose a reason for hiding this comment

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

    This pull request appears to be a cleanup effort, removing unused files and components related to bank functionality, and reorganizing some imports and type definitions. The changes look good overall, with just a couple of minor CSS class duplications that could be cleaned up. The removal of unused code and the consolidation of type definitions should improve code maintainability.

    Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!

    @github-actions github-actions bot added enhancement New feature or request miscellaneous labels Aug 1, 2024
    Copy link

    vercel bot commented Aug 1, 2024

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    eternum ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 7:48pm

    Copy link

    github-actions bot commented Aug 1, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link

    github-actions bot commented Aug 1, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Remove redundant CSS class to clean up the class attribute

    Remove the redundant w-full class from the div element since it is already defined
    in the grid class, improving the readability and maintainability of the class
    attribute.

    client/src/ui/components/military/TroopChip.tsx [14]

    -<div className={`grid grid-cols-3 gap-2 relative w-full text-gold font-bold ${className}`}>
    +<div className={`grid grid-cols-3 gap-2 relative text-gold font-bold ${className}`}>
     
    Suggestion importance[1-10]: 9

    Why: Removing the redundant w-full class improves code readability and maintainability by eliminating unnecessary repetition.

    9
    Improve variable naming for clarity

    Consider using a more descriptive variable name instead of pseudoRandom to clarify
    its purpose and usage within the context.

    client/src/ui/components/construction/GroundGrid.tsx [11]

    -import { ResourceIdToMiningType, getUIPositionFromColRow, pseudoRandom } from "../../utils/utils";
    +import { ResourceIdToMiningType, getUIPositionFromColRow, generatePseudoRandom } from "../../utils/utils";
     
    Suggestion importance[1-10]: 7

    Why: Using a more descriptive variable name like generatePseudoRandom enhances code readability and maintainability, making the purpose of the variable clearer.

    7
    Performance
    Convert ResourceMiningTypes to a const enum for improved performance

    Change the ResourceMiningTypes enum to be a const enum for better performance and to
    prevent it from being compiled into the JavaScript output, which is unnecessary
    since it's only used for type checking.

    client/src/ui/config.tsx [12-15]

    -export enum ResourceMiningTypes {
    +export const enum ResourceMiningTypes {
       Forge = "forge",
       Mine = "mine",
       LumberMill = "lumber_mill",
     }
     
    Suggestion importance[1-10]: 8

    Why: Changing the enum to a const enum can improve performance by preventing unnecessary compilation into JavaScript, which is beneficial since it's only used for type checking.

    8
    Best practice
    Address the TODO by creating a task for refactoring

    Since the comment indicates poor code quality, consider refactoring or at least
    creating a task or issue in your project management tool to address this in the
    future.

    client/src/ui/components/construction/SelectPreviewBuilding.tsx [40]

    -// TODO: THIS IS TERRIBLE CODE, PLEASE REFACTOR
    +// TODO: Refactor this module to improve maintainability and performance. [JIRA-1234]
     
    Suggestion importance[1-10]: 6

    Why: Adding a reference to a task or issue for refactoring is a good practice, but it does not directly improve the code quality in the current PR.

    6

    @edisontim edisontim merged commit b47d5be into main Aug 6, 2024
    21 checks passed
    @edisontim edisontim deleted the enh/cleanup branch August 6, 2024 20:26
    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.

    2 participants