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

Support slash in GitHub branch name #3215

Merged
merged 4 commits into from May 23, 2018

Conversation

Projects
None yet
3 participants
@BlythMeister
Contributor

BlythMeister commented May 21, 2018

This would previously error

Also updated bootstrapper to version including my fix for group policy restrictions

BlythMeister added some commits May 21, 2018

Update paket.exe (magic bootstrapper)
This is the version which installs itself into a paket folder
@matthid

This comment has been minimized.

Member

matthid commented May 21, 2018

The only thing is that a test for this function might make sense (we can make it internal and use InternalsVisibleTo if needed)

@BlythMeister

This comment has been minimized.

Contributor

BlythMeister commented May 21, 2018

Agreed, is there an already existing test fixture?
Also looks like I broke something as both CI gone red 😞

@BlythMeister

This comment has been minimized.

Contributor

BlythMeister commented May 22, 2018

@matthid turns out there is tests (just not for this case) and i broke them :(
Adding my new scenario into that test case and will ensure all tests pass!

@BlythMeister

This comment has been minimized.

Contributor

BlythMeister commented May 22, 2018

@matthid updates made, should be ready to go now...

let config = "github \"owner:project1:master\" \"folder/file.fs\"
github \"owner:project1:feature/branch\" \"folder/file3.fs\"
github \"owner/project1:feature:branch\" \"folder/file4.fs\"

This comment has been minimized.

@matthid

matthid May 22, 2018

Member

Should we really allow that?

This comment has been minimized.

@BlythMeister

BlythMeister May 22, 2018

Contributor

Don't see why not...I can make it blocked if you like though....

This comment has been minimized.

@BlythMeister

BlythMeister May 23, 2018

Contributor

@matthid updated to no longer allow this.
Also added a range of other test cases :)

@forki forki merged commit 9e88075 into fsprojects:master May 23, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@forki

This comment has been minimized.

Member

forki commented May 23, 2018

thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment