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

Run git-standup directly to avoid platform issues #8

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

Conversation

komiga
Copy link
Contributor

@komiga komiga commented Apr 18, 2017

Commits weren't being displayed, so I poked around and found that the standup-helper.sh script wasn't working on my Linux system. It has to be spawned with bash instead of sh to work for me, but I wasn't sure if that'd break it for other platforms. I saw the comment in the script noting that you tried to do it like this before, so this might not work, either.

If this doesn't fly, here's a simple fix for the script.

@notwaldorf
Copy link
Owner

Ooooh thanks! I'm going to take a look at this hopefully tonight!

@mojoaxel
Copy link
Collaborator

This fix by @komiga does not work for me on linux 😢 I'll try to provide a fix.

@komiga
Copy link
Contributor Author

komiga commented Apr 20, 2017

@mojoaxel Oh, I didn't do my due diligence here. The problem was discussed in #14: it's looking for git-standup locally, but it could be installed globally.

I just tried using require.resolve() to find the root path to git-standup regardless of where it's installed, but I think it's not working because the package has no JS (it's just a shell script).

It makes more sense to me to go with a solution like #18, but keeping the behavior of git-standup by searching for repositories in sub-directories, as it's bananas to expect the user to list them all! git-standup does this simply: https://github.com/kamranahmedse/git-standup/blob/master/git-standup#L160

I was going to follow this up by exposing git-standup's maximum search depth option, but it'd have to be built into our search code if we go with git-log.

@mojoaxel
Copy link
Collaborator

I was going to follow this up by exposing git-standup's maximum search depth option, but it'd have to be built into our search code if we go with git-log.

@komiga I did exactly that. It may blow up the code a little but it should work now.

@notwaldorf
Copy link
Owner

notwaldorf commented Apr 20, 2017

@komiga would you mind trying out the latest version and also installing 'npm -g git-standup' like the updated instructions say? I think I found a way to find the path to the correct git-standup (and removed the function keyword which maybe is what was stopping it from working with sh right)

@notwaldorf notwaldorf added the ⏳ pending-response What are you waiting for? Christmas? label Apr 20, 2017
@komiga
Copy link
Contributor Author

komiga commented Apr 29, 2017

@notwaldorf Sorry for the super late reply! The current master branch works fine for me when I have git-standup installed globally.

Do you want to drop this PR and focus on #18? It looks like it's converging on a final solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏳ pending-response What are you waiting for? Christmas?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants