-
-
Notifications
You must be signed in to change notification settings - Fork 5
Stable config sort #226
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
Merged
Merged
Stable config sort #226
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement alphabetical sorting of repositories in the .gitopolis.toml configuration file. Repos are now sorted by path (a-z) when added. Implementation approach: Initially attempted to sort in Repos::as_vec() to have a single source of truth for sorting. However, this approach had issues: - Changing as_vec() from returning &Vec to Vec required cascading changes - serialize() needed extra work to call as_vec() and rebuild Repos - Added unnecessary complexity and allocations Final approach sorts in-place at the point of mutation (add methods). This is cleaner because: - Serde serializes self.repos directly, so it needs to already be sorted - Sorting once per add is negligible performance impact - No signature changes needed to as_vec() or other methods - Simpler code with sorting co-located with mutation Changes: - Added private add_repo() method in Repos that pushes and sorts repos - Modified add() and add_with_tags_and_remotes() to use add_repo() (DRY) - Repos are sorted in-place after each addition - Added end-to-end test repos_stored_alphabetically() verifying repos added in reverse order are stored alphabetically This is the first part of #225 - repository sorting only. Tag sorting will be implemented separately. Prompts: - #225 - make the sorting of repos and their tags a-z in the stored gitopolis config file. tdd it by adding two repos and tags in reverse order - if state.contains - is completely redundant - we're checking the entire toml - [Request interrupted by user]you're overcomplicated it - use only alpha and zulu, put zulu in the initial config then add alpha - [Request interrupted by user for tool use] - [Request interrupted by user]right, closer. the tests should be end to end, not unit. the sorting is in the wrong place, it's been duplicated in serialze but it should live in a single place. I've stashed the changes. let's do repos first and come back to tags. Repos.as_vec() would probably be a decent place to apply the sort as we have a very short-lived single-operation lifetime for the entire program. - [Request interrupted by user for tool use] - you aren't listening, I said only repos for now - what on earth is going on with the diff in serialize()? that shouldn't need any changes surely? - we could do the sorting in add() - [Request interrupted by user for tool use]DRY add() and add_with_tags_and_remotes() - [Request interrupted by user for tool use]update the commit description to match the updated diff from origin/main - [Request interrupted by user for tool use]the commit needs to explain WHY we ended up doing the sort in add, and what we tried that didn't work out Co-Authored-By: Claude <noreply@anthropic.com>
Ensures that tags within each repository are stored in alphabetical order (a-z) in the .gitopolis.toml configuration file. Tags are now sorted at the point of mutation when they are added to a repository. This complements the repository sorting implemented previously and provides a consistent alphabetical ordering throughout the configuration. Implementation: - Sort tags immediately after adding them by calling `repo.tags.sort()` in the `tag()` method - Follows same mutation-point sorting pattern as repository sorting - Existing test `list_long` updated to expect sorted tag order Test coverage: - Added end-to-end test `tags_stored_alphabetically()` that adds tags in reverse order (zulu, alpha) and verifies they are stored sorted (alpha, zulu) - Updated `list_long` test expectation from "some_tag,another_tag" to "another_tag,some_tag" Prompts: - lovely jubbly. now make tag sorting stable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Fixes #225
Changed repository and tag sorting to be case-insensitive by converting strings to lowercase before comparison. This provides more intuitive alphabetical ordering where "alpha", "Beta", "zulu" sorts correctly regardless of capitalization. Implementation: - Updated `Repos::add_repo()` to use `sort_by()` with `to_lowercase().cmp()` - Updated `Repos::tag()` to use `sort_by()` with `to_lowercase().cmp()` Test changes (TDD): - Extended `repos_stored_alphabetically` test to include mixed-case "Beta_repo" - Expected order: alpha_repo < Beta_repo < zulu_repo (case-insensitive) - Extended `tags_stored_alphabetically` test to include mixed-case "Beta" tag - Expected order: alpha < Beta < zulu (case-insensitive) All 76 tests pass. Prompts: - expand the repo/tag sort tests to show they are case-insensitive - [Request interrupted by user for tool use]um, that's not what I said, I said test for *insensitive*. I know it'll fail. TDD. duh. - [Request interrupted by user for tool use]ALL the prompts, as per instructions. goldfish 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> #225
Updated three unit tests in gitopolis_tests.rs to include mixed-case examples, verifying that repository sorting is case-insensitive throughout the system. Also fixed the `list()` method in gitopolis.rs to use case-insensitive sorting, which was previously using case-sensitive sorting and causing the updated tests to fail. Changes: - list_returns_repos_sorted_alphabetically: Added Beta_repo to verify alpha < Beta < zebra - list_with_tag_filter_returns_sorted_repos: Added Beta_repo with backend tag - tags_long_repos_sorted_within_tag: Added Beta_repo with backend tag - Fixed Gitopolis::list() to use .to_lowercase().cmp() for case-insensitive sorting All 76 tests pass. Prompts: - list_returns_repos_sorted_alphabetically list_with_tag_filter_returns_sorted_repos tags_long_repos_sorted_within_tag - all need prove case-insensitive sorting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
caf5bfe to
2df1816
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pr for ci