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

Fix duplicate declaration of grm, update grb<*> #2220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bittner
Copy link
Contributor

@bittner bittner commented Oct 1, 2023

Description

Consolidates the git rebase aliases thus fixing the unintended overloading of grm. gdel is outphased as an unclear, non-intuitive sibling of gbD (git branch --delete --force).

Motivation and Context

grm is an alias for git rebase due to being redeclared after the first grm (git rm). The redeclaration is a bug.

Closes #2160.

How Has This Been Tested?

n/a

Screenshots (if appropriate):

n/a

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

@bittner
Copy link
Contributor Author

bittner commented Oct 1, 2023

This PR is a follow-up on #2183, which was accidentally closed. See there and #2160 for clarifications, as the changes were discussed in depth.

The failing of the lint job is unrelated to the changes introduced in this PR.

@bittner
Copy link
Contributor Author

bittner commented Nov 2, 2023

This PR fixes a bug. The changes have been extensively discussed.

The only thing missing it someone to press the Merge button. Please? 🙏

@bittner
Copy link
Contributor Author

bittner commented Nov 5, 2023

Can anyone clarify why for macos-11 tests are skipped, which makes the test job fail? 52 tests are skipped: (3 + 46 + 3)

  • 16-18 (alias-completion)
  • 19-64 (completion bash-it)
  • 71-73 (uninstall)
Log excerpt (macos-11)
...
ok 15 bash-it: load enabled aliases from old structure, without priorities
ok 65 install: verify that the install script exists
...
ok 70 install: verify that the template is appended
ok 74 lib composure: _composure_keywords()
...
Skipped tests (macos-11)
ok 16 alias-completion: See that aliases with double quotes and brackets do not break the plugin
ok 17 alias-completion: See that aliases with single quotes and brackets do not break the plugin
ok 18 alias-completion: See that having aliased rm command does not output unnecessary output
ok 19 completion bash-it: ensure that the _bash-it function is available
ok 20 completion bash-it: doctor - show options
ok 21 completion bash-it: help - show options
ok 22 completion bash-it: help - aliases v
ok 23 completion bash-it: update - show options
ok 24 completion bash-it: update - show optional flags
ok 25 completion bash-it: search - show no options
ok 26 completion bash-it: migrate - show no options
ok 27 completion bash-it: show options
ok 28 completion bash-it: bash-ti - show options
ok 29 completion bash-it: shit - show options
ok 30 completion bash-it: bashit - show options
ok 31 completion bash-it: batshit - show options
ok 32 completion bash-it: bash_it - show options
ok 33 completion bash-it: profile - show options
ok 34 completion bash-it: profile load - show options
ok 35 completion bash-it: show - show options
ok 36 completion bash-it: enable - show options
ok 37 completion bash-it: enable - show options a
ok 38 completion bash-it: disable - show options
ok 39 completion bash-it: disable - show options a
ok 40 completion bash-it: disable - provide nothing when atom is not enabled
ok 41 completion bash-it: disable - provide all when atom is not enabled
ok 42 completion bash-it: disable - provide the a* aliases when atom is enabled with the old location and name
ok 43 completion bash-it: disable - provide the a* aliases when atom is enabled with the old location and priority-based name
ok 44 completion bash-it: disable - provide the a* aliases when atom is enabled with the new location and priority-based name
ok 45 completion bash-it: disable - provide the docker-machine plugin when docker-machine is enabled with the old location and name
ok 46 completion bash-it: disable - provide the docker-machine plugin when docker-machine is enabled with the old location and priority-based name
ok 47 completion bash-it: disable - provide the docker-machine plugin when docker-machine is enabled with the new location and priority-based name
ok 48 completion bash-it: disable - provide the todo.txt-cli aliases when todo plugin is enabled with the old location and name
ok 49 completion bash-it: disable - provide the todo.txt-cli aliases when todo plugin is enabled with the old location and priority-based name
ok 50 completion bash-it: disable - provide the todo.txt-cli aliases when todo plugin is enabled with the new location and priority-based name
ok 51 completion bash-it: enable - provide the atom aliases when not enabled
ok 52 completion bash-it: enable - provide the a* aliases when not enabled
ok 53 completion bash-it: enable - provide the a* aliases when atom is enabled with the old location and name
ok 54 completion bash-it: enable - provide the a* aliases when atom is enabled with the old location and priority-based name
ok 55 completion bash-it: enable - provide the a* aliases when atom is enabled with the new location and priority-based name
ok 56 completion bash-it: enable - provide the docker* plugins when docker-compose is enabled with the old location and name
ok 57 completion bash-it: enable - provide the docker-* plugins when docker-compose is enabled with the old location and priority-based name
ok 58 completion bash-it: enable - provide the docker-* plugins when docker-compose is enabled with the new location and priority-based name
ok 59 completion bash-it: enable - provide the docker* completions when docker-compose is enabled with the old location and name
ok 60 completion bash-it: enable - provide the docker* completions when docker-compose is enabled with the old location and priority-based name
ok 61 completion bash-it: enable - provide the docker* completions when docker-compose is enabled with the new location and priority-based name
ok 62 completion bash-it: enable - provide the todo.txt-cli aliases when todo plugin is enabled with the old location and name
ok 63 completion bash-it: enable - provide the todo.txt-cli aliases when todo plugin is enabled with the old location and priority-based name
ok 64 completion bash-it: enable - provide the todo.txt-cli aliases when todo plugin is enabled with the new location and priority-based name
ok 71 uninstall: verify that the uninstall script exists
ok 72 uninstall: run the uninstall script with an existing backup file
ok 73 uninstall: run the uninstall script without an existing backup file

Is this relevant and needs fixing, or is this an operational error or a flaky test suite?

@cornfeedhobo
Copy link
Member

@bittner I don't have an answer for why the macos tests do this, but it is my understanding that yes, they need fixing.

@bittner
Copy link
Contributor Author

bittner commented Nov 7, 2023

Can we first merge all the other git-alias related PRs? My hope is that after rebasing onto those changesets the tests run successfully, PRs like this one either succeed (by magic, i.e. we have flaky tests) or the reasons for failing will become clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Duplicate declaration of grm in git.aliases.bash
2 participants