-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Added composure as a submodule, replaced composure.bash with a symlink #872
Conversation
- Removed "normal" file lib/composure.bash - Symlinked lib/composure/composure.sh to lib/composure.bash - This allows ease of updating composure directly from git.
…nt. Including composure as a submodule means that cloning this repository must be done recursively.
Thanks for contributing! I like the idea of this, but have a couple of questions:
|
Hm the HTTPS idea sounds good. I was wondering why it wouldn't build on Travis, but switching it over to HTTPS should fix it. (This was my first time contributing to something tracked by travis, though, so I'm hoping we're right on that). I am not 150% sure if a straight Worst case though, except for someone who customized bash-it's composure.bash file it would only require an extra one or two git commands. The reason I made a symlink |
Changed composure remote origin to https://github.com/erichs/composure.git (hopefully travis likes https)
…modules. (*really* changed the submodule URL)
I just created a "test" branch on my repo identical to your "master" branch, added a new user to my box (to start fresh), installed bash-it for that user from my "test" branch, merged my "master" branch into "test" to simulate the merging of this pull request with your master, and updated bash-it against that branch. Everything worked fine, as I expected. I actually did not even need to use any recursive or submodule commands with git — it knew what to do with the new submodule. It should be fine. As far as I can see it, the only way someone would be able to modify their own copy of bash-it in such a way that this change would break their install would be if they actually customized And even if someone actually did mess with their local copy of the composure library, I think git would realize the hashes don't match and not update composure.bash itself (simply adding the submodule and not the symlink). If that's not the case, that's the fault of Also, I think the reason that the SSH URL was failing on travis-ci is because if a GitHub user is logged into an account with SSH keys on file, clicking a clone button gives you a link that uses those keys — I agree with you that HTTPS makes sense in case of firewalls, etc., but I'm glad I figured out what the problem with Travis is. |
Sorry, you'll have to fix one more thing... The Travis build is showing green, but it's not executing any of the test cases: https://travis-ci.org/Bash-it/bash-it/builds/188593255 There's a |
Sure, I'll check it out soon as I get to my desk today. Is it straight
bash? I've only ever seen .bats in bash-it
…On Wed, Jan 4, 2017 at 2:45 AM Nils Winkler ***@***.***> wrote:
Sorry, you'll have to fix one more thing... The Travis build is showing
green, but it's not executing any of the test cases:
https://travis-ci.org/Bash-it/bash-it/builds/188593255
There's a composure.bats file in test/lib, which is running some checks
on the composure functionality, and it looks like the submodule or the
symbolic link broke that test (and all of the other tests). Can you try to
run the tests locally to see whether you get the same issue?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#872 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AARYdXMh9HK-REPyoyTdizrftqBfnVQIks5rO037gaJpZM4LXzj7>
.
|
It is using this for the tests: https://github.com/sstephenson/bats |
Great, thanks! I'll check it out and we can figure out what's wrong. :)
…On Wed, Jan 4, 2017 at 8:18 AM Nils Winkler ***@***.***> wrote:
It is using this for the tests: https://github.com/sstephenson/bats
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#872 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AARYdY7SUkC2xWQbalTjxW0q1OTcryB4ks5rO5wNgaJpZM4LXzj7>
.
|
I see why it's failing. So, bats likes neither symlinks, nor a stub |
Thanks for trying to get this to work! Let me know when you feel that this is ready for another look... |
Merge with bash-it/bash-it head
If this is to be picked up again, there are few more changes that need to be done Line 621 in 45ae151
Line 667 in 45ae151
have to be changed to Line 628 in 45ae151
to |
@nwinkler @b- @ahmadassaf I think we can close this PR after the merge of #1820 |
This project incorporates composure by @erichs, so I replaced its copy of composure with a submodule linking to his composure repository. The official composure repo, however, names the main file
composure.sh
when bash-it is looking forcomposure.bash
in its libs folder, so I replacedcomposure.bash
with a symlink pointing tocomposure/composure.sh