-
Notifications
You must be signed in to change notification settings - Fork 94
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
Remove the underscore prefix of __bp_imported
#123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @FranklinYu! Looks like there's a couple other places that use the original name:
[rcaloras:~/git/bash-preexec] master ± git grep __bp_imported
...
test/include-test.bats: __bp_imported="defined"
test/include-test.bats: unset __bp_imported
Can you also the test case for this in to use the new variable name?
All of the suggestion above looks good to me, but apparently the test is currently broken. For example, the “include test” use |
This formally exposes it as part of the public API.
5d4b1e6
to
7fbc07f
Compare
I’m glad that the CI is working, and it caught a bug. I’ll fix it before marking this as “ready for review”. |
Variables were reanmed in 7fbc07f, but it didn’t cover the test or ReadMe. This commit fixes that.
I finally addressed all the comments. The history seems a bit messy; maybe consider merge by squashing the commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This formally exposes it as part of the public API. RFC: @dimo414.
Fixes #122.