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

small cleanup #1147

Closed
wants to merge 50 commits into from
Closed

small cleanup #1147

wants to merge 50 commits into from

Conversation

bob0005
Copy link
Collaborator

@bob0005 bob0005 commented Jul 30, 2024

PR Type

enhancement, miscellaneous


Description

  • Removed redundant CSS classes from div elements in TroopChip and BattleDetails components.
  • Cleaned up unused imports in StructureConstructionMenu.
  • Removed redundant ResourceMiningTypes enum definition and imported it from utils/utils in config.tsx.
  • Removed unused armiesAtPosition variable in BattleView.

Changes walkthrough 📝

Relevant files
Enhancement
TroopChip.tsx
Remove redundant CSS class in TroopChip component               

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

  • Removed redundant w-full class from the div element.
+1/-1     
config.tsx
Remove redundant enum and import from utils in config       

client/src/ui/config.tsx

  • Removed redundant ResourceMiningTypes enum definition.
  • Imported ResourceMiningTypes from utils/utils.
  • +1/-7     
    BattleDetails.tsx
    Remove redundant CSS class in BattleDetails component       

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

    • Removed redundant p-2 class from the div element.
    +1/-1     
    Miscellaneous
    StructureConstructionMenu.tsx
    Clean up unused imports in StructureConstructionMenu         

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

    • Removed unused imports to clean up the code.
    +0/-9     
    BattleView.tsx
    Remove unused variable in BattleView component                     

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

    • Removed unused armiesAtPosition variable.
    +0/-2     

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

    Copy link

    vercel bot commented Jul 30, 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 1, 2024 9:16am

    Copy link
    Contributor

    @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.

    Overall, this PR contains good cleanup efforts. It removes duplicate CSS classes, unused imports, and unnecessary code. These changes, while small, contribute to a cleaner and more maintainable codebase. The changes are straightforward and don't introduce any new functionality or potential bugs. Good job on the cleanup!

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

    Copy link

    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 Jul 30, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Remove redundant CSS class to clean up the class string

    Consider removing the redundant w-full class in the div element since it is already
    included in the class string.

    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]: 8

    Why: The suggestion correctly identifies a redundant CSS class (w-full) in the div element, which improves code cleanliness and maintainability without affecting functionality.

    8
    Maintainability
    Verify the usage of imported modules to avoid unused imports

    Ensure that the ResourceMiningTypes import is used within the file, or consider
    removing it if it's unused to keep the codebase clean.

    client/src/ui/config.tsx [2]

    -import { ResourceMiningTypes } from "./utils/utils";
    +// Ensure usage or remove if unused
     
    Suggestion importance[1-10]: 7

    Why: The suggestion is valid as it encourages checking for unused imports, which helps in keeping the codebase clean and efficient. However, it does not provide a direct improvement but rather a recommendation.

    7
    Best practice
    Use consistent import formatting for better readability and maintainability

    Consider using object destructuring for imports from @bibliothecadao/eternum to make
    the import statement cleaner and more consistent.

    client/src/ui/components/structures/construction/StructureConstructionMenu.tsx [3-6]

    -import {
    -  EternumGlobalConfig,
    -  HYPERSTRUCTURE_POINTS_PER_CYCLE,
    -  STRUCTURE_COSTS_SCALED,
    -  StructureType,
    -} from "@bibliothecadao/eternum";
    +import { EternumGlobalConfig, HYPERSTRUCTURE_POINTS_PER_CYCLE, STRUCTURE_COSTS_SCALED, StructureType } from "@bibliothecadao/eternum";
     
    Suggestion importance[1-10]: 6

    Why: The suggestion promotes better readability and maintainability by using object destructuring for imports. While it is a good practice, it is a minor improvement and does not significantly impact the code functionality.

    6

    edisontim and others added 2 commits July 31, 2024 18:46
    * Change to V1 + add type aliases for entity ids
    
    * Make elements clickable under right and left navigation modules buttons + fix z-index of quests, guilds and leaderboard
    
    * scarb fmt
    
    * Change bonus_percent to u32 instead of u128
    
    * Address minor PR comments
    
    * prettier
    
    * knip
    Base automatically changed from v1 to main August 1, 2024 12:14
    @bob0005 bob0005 closed this Aug 1, 2024
    @bob0005 bob0005 deleted the clean-up branch August 1, 2024 15:59
    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.

    4 participants