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

Optimal-viewing-position experiment #88

Closed
wants to merge 1 commit into from

Conversation

smathot
Copy link
Collaborator

@smathot smathot commented Jul 12, 2018

I tested this with the latest revamp (#84). The experiment hangs without crashing upon launching it.

@smathot smathot requested a review from dschreij July 12, 2018 10:09
@dschreij
Copy link
Collaborator

I'll have a look soon!

@dschreij
Copy link
Collaborator

I tried to checkout your PR branch directly, but did you use a very old version/commit of osweb by any chance? I could not get the app built. I ended up simply copying the ovp.osexp file to my master branch, and then I could indeed load and start the experiment, but get nothing more than a black screen (and no error messages in the console, alas). So the first challenge has arrived!

Copy link
Collaborator

@dschreij dschreij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR is based on an old commit of osweb? Anyway, the experiment can be extracted and opened, so the important part is ok.

@smathot
Copy link
Collaborator Author

smathot commented Jul 13, 2018

I tested this with the latest changes, i.e. the state of #84. However, to keep the PR clean I reset the state to shyras/master and sent only the experiment commit in this PR.

@smathot
Copy link
Collaborator Author

smathot commented Jul 18, 2018

The PR still says changes requested. But what kind of changes do you mean?

@dschreij
Copy link
Collaborator

There are still some discrepancies with osweb/master from shyras and the contents of your PR (or the state of the repo it references to). For example, your repo still contains the test folder, which I removed somewhere during the revamp. I thus couldn't easily merge your changes into my master branch, because I got all kinds of weird conflicts.

I didn't feel like solving the batch of git conflicts, so I solved this by just copying the ovp.osexp file in my own repo, since this was the only change (right?).

@dschreij
Copy link
Collaborator

dschreij commented Jul 18, 2018

You can also see the difference in your README.md. It still shows the old build commands. Somehow the branch you sent the PR from was not completely updated, I guess.

@smathot
Copy link
Collaborator Author

smathot commented Jul 18, 2018

It's a clean pull request relative to shyras/master, just not relative to your branch, probably ;-) But no matter, if you've added it manually you can close this PR. Another trick in these cases that I often use is git cherry-pick to select only the commit that I need.

@dschreij dschreij closed this Aug 14, 2018
@smathot smathot deleted the ovp_example branch April 29, 2020 08:06
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.

None yet

2 participants