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

Rename getPath() to getRealPath() and getAllPaths() to getAllRealPaths() #8

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

Rename getPath() to getRealPath() and getAllPaths() to getAllRealPaths() #8

wants to merge 1 commit into from

Conversation

lenton
Copy link
Contributor

@lenton lenton commented Feb 2, 2015

This is a much more descriptive name for the two methods rather than the generic 'getPath'. It makes it much clearer that these methods get the real file path from their virtual one.

@lenton
Copy link
Contributor Author

lenton commented Feb 2, 2015

It seems HHVM doesn't work with vfsStream and it's not going to be fixed any time soon: facebook/hhvm#1971

I guess we're going to have to remove the HHVM travis check for now, unless anyone knows a work around?

@lenton
Copy link
Contributor Author

lenton commented Feb 2, 2015

I've made it so HHVM is now not required to pass its build: 1b2e619

Although when restarting the build it doesn't take into account the change, so you'll just have to ignore the failure for this PR.

@acoulton
Copy link
Member

acoulton commented Feb 3, 2015

@lenton how very dull. The only workaround I can think of would be to drop vfsStream and make a temporary directory structure in the sys_get_temp_dir but that's not great either. An idealistic solution would use vfsStream on other platforms and a real filesystem on HHVM I guess...

As is, I think we should just skip the known bad tests - otherwise in future something we care about could fail on HHVM but the commit could still get a green tick. See #9

To get Travis to consider the changed configuration (either yours in master or mine), you'd need to rebase this branch - just restarting the build will build the same commit with the same travis configuration.

@lenton
Copy link
Contributor Author

lenton commented Feb 3, 2015

@acoulton Thanks, that's a better work around.

To get Travis to consider the changed configuration (either yours in master or mine), you'd need to rebase this branch - just restarting the build will build the same commit with the same travis configuration.

😞 How silly of me, that never crossed my mind.

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.

2 participants