Skip to content
This repository has been archived by the owner on May 13, 2020. It is now read-only.

Fix invalid separator in PONYPATH for windows. #32

Merged
merged 1 commit into from
Aug 9, 2017

Conversation

ta3ta1
Copy link
Contributor

@ta3ta1 ta3ta1 commented Aug 9, 2017

When it has multiple dependencies, PONYPATH end up like

PONYPATH=C:\.deps\foo:C:\.deps\bar

But PONYPATH should be separated by semicolon on windows.

@SeanTAllen
Copy link
Member

@ta3ta1 thanks for this.

2 things:

1 can you add a CHANGELOG entry under Fixed that says:

Fix invalid separator in PONYPATH for Windows


can you update the commit comment to include what you put in GitHub?

When it has multiple dependencies, PONYPATH end up like

PONYPATH=C:\.deps\foo:C:\.deps\bar

But PONYPATH should be separated by semicolon on windows.

That will make the commit much more understandable in the future.

--

You can do both at one by making your CHANGELOG change then doing

  • git add CHANGELOG.md
  • git commit --amend

this will allow you to update your previous commit comment to include the description here plus include the CHANGELOG entry with it.

  • git push --force-with-lease

Will update this PR with your changed history (changing the comment is a history change as is adding another file to the commit)


THANKS!

When it has multiple dependencies, PONYPATH end up like

`PONYPATH=C:\.deps\foo:C:\.deps\bar`

But PONYPATH should be separated by semicolon on Windows.
@ta3ta1
Copy link
Contributor Author

ta3ta1 commented Aug 9, 2017

Updated commit.

@ta3ta1
Copy link
Contributor Author

ta3ta1 commented Aug 9, 2017

Thanks for navigation btw.
I didn't know that options and was afraid of things like amend/force after push.

@SeanTAllen
Copy link
Member

I'm merging this assuming it works. @ponylang/committer we need to think about what sort of automated tests we want for stable.

@SeanTAllen
Copy link
Member

@ta3ta1 thank you!

@SeanTAllen SeanTAllen merged commit 7482c4f into ponylang:master Aug 9, 2017
@ta3ta1 ta3ta1 deleted the fixsep branch August 9, 2017 17:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants