Skip to content

Remove map structs#490

Merged
tsmbland merged 10 commits intomainfrom
maps_alternative
Apr 23, 2025
Merged

Remove map structs#490
tsmbland merged 10 commits intomainfrom
maps_alternative

Conversation

@tsmbland
Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland commented Apr 16, 2025

Description

Turning existing map structs into type aliases. Removes a lot of code, with only minor refactoring required

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change to fix an issue)
  • New feature (non-breaking change to add functionality)
  • Refactoring (non-breaking, non-functional change to improve maintainability)
  • Optimization (non-breaking change to speed up the code)
  • Breaking change (whatever its nature)
  • Documentation (improve or add documentation)

Key checklist

  • All tests pass: $ cargo test
  • The documentation builds and looks OK: $ cargo doc

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.03%. Comparing base (28972b7) to head (f76027b).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #490      +/-   ##
==========================================
- Coverage   95.03%   95.03%   -0.01%     
==========================================
  Files          36       36              
  Lines        4914     4893      -21     
  Branches     4914     4893      -21     
==========================================
- Hits         4670     4650      -20     
  Misses        123      123              
+ Partials      121      120       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tsmbland tsmbland mentioned this pull request Apr 16, 2025
10 tasks
@tsmbland tsmbland changed the title Alternative to #489 Alternative implementation for maps Apr 16, 2025
@tsmbland tsmbland requested a review from alexdewar April 16, 2025 15:51
@tsmbland tsmbland mentioned this pull request Apr 23, 2025
10 tasks
@tsmbland tsmbland added this to MUSE Apr 23, 2025
@tsmbland tsmbland self-assigned this Apr 23, 2025
@tsmbland tsmbland changed the title Alternative implementation for maps Remove map structs Apr 23, 2025
Copy link
Copy Markdown
Member

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

I think I prefer this to #489 (although that is ok as a solution too). It's just a bit simpler. The only downside is that some things look a bit uglier with extra &s and clone()s, but I'm not sure it's worth having custom structs just to avoid that.

@tsmbland
Copy link
Copy Markdown
Collaborator Author

Ok thanks, I'll go ahead with this approach then, and give it a go for the remaining maps still left to implement. Can always switch back to custom structs later if we feel it's necessary, but I doubt it

@tsmbland tsmbland marked this pull request as ready for review April 23, 2025 12:06
@tsmbland tsmbland enabled auto-merge April 23, 2025 12:07
@tsmbland tsmbland merged commit 043e63e into main Apr 23, 2025
7 checks passed
@tsmbland tsmbland deleted the maps_alternative branch April 23, 2025 12:08
@github-project-automation github-project-automation Bot moved this to ✅ Done in MUSE Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants