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

Update gitexec to v2.0.1 #66

Closed
wants to merge 1 commit into from
Closed

Update gitexec to v2.0.1 #66

wants to merge 1 commit into from

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Oct 4, 2019

Currently, this breaks Windows testing. More specifically this patch: rvagg/gitexec@51ad38d

This PR is split from #61 so that we tackle the Windows issue separately.

Closes #54

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 4, 2019

@rvagg: this is clearly broken and we need to fix it, why approve it :P

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 4, 2019

Alright, after seeing the source code, I don't think changelog-maker can ever work on Windows native cmd...

I'm not sure how to make all of those posix stuff work on Windows like

$(git show -s --format=%cd `{{refcmd}}`)

2> /dev/null could probably be 2>NUL on Windows and it should work.

So, if after spending some time @rvagg you don't think we can make it work nice on Windows native cmd, we should revert the gitexec shell change and release 2.0.1 and then update to 2.0.1 here.

We'll just need to list that changelog-maker requires bash.

I managed to test the master branch on my Windows VM with msys2

xmr@DESKTOP-2989SN9 MSYS /c/Users/xmr/Desktop/changelog-maker
$ npm run unit

> [email protected] unit C:\Users\xmr\Desktop\changelog-maker
> tape test.js

TAP version 13
# test basic commit block
ok 1 should be equal
# test filter-release
ok 2 should be equal
# test simple
ok 3 should be equal
# test group, semver labels, PR-URL
ok 4 should be equal
# test simple group, semver labels, PR-URL
ok 5 should be equal
# test blank commit-url
ok 6 should be equal
# test blank commit-url
ok 7 should be equal

1..7
# tests 7
# pass  7

# ok

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 4, 2019

Unless, we split the subcommands and we joined them thus skipping backticks and ${}. The rest should work fine.

It might be doable.

@rvagg
Copy link
Member

rvagg commented Oct 14, 2019

yeah, it's going to be the $() bash subcommands that are the problem on windows, but we could pull them apart pretty easily.

const commitdatecmd = '$(git show -s --format=%cd {{refcmd}})' is the offending piece, used for both sincecmd and untilcmd, we could get rid of the $() around that and run them separately with gitexec(), store the output of both and then insert them back into the parent git log.

@XhmikosR
Copy link
Contributor Author

@rvagg sounds good, can you push your suggestion on this branch and ping me?

@rvagg
Copy link
Member

rvagg commented Oct 14, 2019

I started playing with this today but it's way more complex than I imagined! there's a lot of bash subcommand nesting going on in the original structure that's a bit awkward to pull apart. An interesting challenge though; but very low priority so no guarantees that it's going to get done any time soon.

@XhmikosR XhmikosR changed the title Update gitexec to v2.0.0 Update gitexec to v2.0.1 Oct 15, 2019
@rvagg rvagg closed this Oct 16, 2019
@rvagg rvagg deleted the xmr-pack-gitexec-up branch October 16, 2019 09:58
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.

Some warnings
2 participants