jpaulm / jsfbp

FBP implementation written using JavaScript and node-fibers
MIT License
119 stars 23 forks source link

Need to reinstate send suspend logic #78

Open jpaulm opened 7 years ago

jpaulm commented 7 years ago

Need to reinstate send suspend logic - this seems to be mostly in PR #74

dancrumb commented 7 years ago

I want to make sure I understand the logic that you're talking about.

This is the logic that suspends a receiver when there are no IPs currently in any incoming connections to the InputPort that the receiver has called receive on. Is that correct?

dancrumb commented 7 years ago

On further investigation, I see that the whole receive code is broken, because there's no support for multiple connections coming into an InputPort.

If you have the patience, I'd like to spend some time working on resolving this. It will probably take a few days. I'll work on a branch so that there is a reviewable Pull Request.

If you're already working on a fix, just let me know.

jpaulm commented 7 years ago

Re the post before your last one: what seems to have disappeared is the logic to suspend a sender when the connection being sent to is full. If I'm right, this would explain why the app you were working on saturates storage. I referred you to the discussion of ballooning in the book - this is not a desirable behaviour, as logic errors can behave like storage errors, and the application behaviour becomes much less predictable.

In this case you can look at the code before PR #74 was merged in to see how connections and the related ports are interrelated.

jpaulm commented 7 years ago

Re your last post, I am happy to let you do this. I have reset my personal version of jsfbp to the one before PR #74 was merged in. My initial test worked fine, and I could happily run with this version. However it is rather a sledge-hammer approach, and would cause jsfbp to lose some doubtless useful changes that you put in later. So if you can be more surgical about removing the effects of PR #74 on the connection logic, that would be much better.

However, I want to say that I really dislike people refactoring my code, and moving stuff around, as it makes it nearly impossible for me to see what has changed. Sorry if this offends your esthetic sense :-) , but my classes are chosen rather carefully, and I don't like to see them changed in any significant way. In this case your refactoring seems to have led to quite a bit of confusion - on both our parts!

See also my earlier comment about how the Travis test didn't pick up on this at all, leading me to think everything was fine... when it wasn't!

You are actually the 3rd person to try refactoring various implementations - first the JavaFBP implementation some years ago, then the C#FBP one, and now jsfbp. In both of the previous cases, it became obvious that the people involved were confused at a very fundamental level, so we had to give up on the effort!

Hope we can continue to work together - there is a lot of good stuff here!

jpaulm commented 7 years ago

Hi Dan,

How is this going? I realized I should have posted a warning on the Readme when I found about this. GitHub seems to be slow right now, but I will do it as soon as possible - unless I hear from you differently.

Regards,

Paul

On Fri, Nov 11, 2016 at 4:51 PM, Dan Rumney notifications@github.com wrote:

On further investigation, I see that the whole receive code is broken, because there's no support for multiple connections coming into an InputPort.

If you have the patience, I'd like to spend some time working on resolving this. It will probably take a few days. I'll work on a branch so that there is a reviewable Pull Request.

If you're already working on a fix, just let me know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-260063826, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ3sJ_rpKPDNHUwFOReUpNibK4mdLks5q9ONagaJpZM4KvycM .

jpaulm commented 7 years ago

PS In my npm test breader seems to have broken - can I fix it, or drop it from npm test? If the latter, where are the tests stored?

Thx

Paul

On Wed, Nov 16, 2016 at 9:54 AM, Paul Morrison jpaulmorr@gmail.com wrote:

Hi Dan,

How is this going? I realized I should have posted a warning on the Readme when I found about this. GitHub seems to be slow right now, but I will do it as soon as possible - unless I hear from you differently.

Regards,

Paul

On Fri, Nov 11, 2016 at 4:51 PM, Dan Rumney notifications@github.com wrote:

On further investigation, I see that the whole receive code is broken, because there's no support for multiple connections coming into an InputPort.

If you have the patience, I'd like to spend some time working on resolving this. It will probably take a few days. I'll work on a branch so that there is a reviewable Pull Request.

If you're already working on a fix, just let me know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-260063826, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ3sJ_rpKPDNHUwFOReUpNibK4mdLks5q9ONagaJpZM4KvycM .

dancrumb commented 7 years ago

Hi Paul,

Went away for the weekend and been catching up.

Regarding suspending the sender, I believe this remains in place in the ProcessConnection code, specifically at: https://github.com/jpaulm/jsfbp/blob/master/core/ProcessConnection.js#L79

At this point, if the number of pending IPs is greater than the connection capacity, then the process is put into WAITING_TO_SEND status and is not requeued for the runtime; then, the process yields, thereby suspending it until it's added to the runtime queue again.

This check occurs in a permanent loop that isn't broken until the IP that is being 'sent' can be added to the connection.

The absence of tests around this behaviour is definitely an oversight and a problem. Perhaps the first step is to try and craft a test that passes when the behaviour is correct and otherwise fails.

Certainly Travis didn't catch this, but I think that's more a shortcoming of the test suite rather than an inherent shortcoming of CI.

I think an update to the README is probably a good idea. Alternatively, another approach might be to branch off the current master branch and then revert all of the changes I put in, with a view to being more circumspect about the changes.

Finally, you can modify https://github.com/jpaulm/jsfbp/blob/master/test/components/breader.js#L9 to read describe.skip to skip that test case. I think it's better to 'skip' than remove, since you have that reminder that you have a test that's not currently working without it breaking your workflow.

dancrumb commented 7 years ago

As I look at it, you should roll my work back... my implementation of connections is way off.

My ProcessConnection class has a single capacity, but multiple upstream processes... this is a totally incorrect representation of the way that multiple OutputPorts can fan into an InputPort.

jpaulm commented 7 years ago

Thanks, Dan, it would be best for now. A few days ago I did a git reset --hard to the commit before your big merge, and npm test works, except for breader.

When I do a git status, I get:

C:\Users\Paul\Documents\GitHub\jsfbp [master ↓]> git status On branch master Your branch is behind 'origin/master' by 28 commits, and can be fast-forwarded. (use "git pull" to update your local branch) nothing to commit, working tree clean

What is the correct git statement to set the repo to this level for the time being?

Thanks,

Paul

On Wed, Nov 16, 2016 at 12:02 PM, Dan Rumney notifications@github.com wrote:

As I look at it, you should roll my work back... my implementation of connections is way off.

My ProcessConnection class has a single capacity, but multiple upstream processes... this is a totally incorrect representation of the way that multiple OutputPorts can fan into an InputPort.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261005604, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ8v7vC3l-JOe3GMoUlNUP2a5p-FXks5q-zc0gaJpZM4KvycM .

jpaulm commented 7 years ago

No that's correct - the ProcessConnection has the capacity, as it is where in transit IPs are held. OutputPorts only have a "closed" attribute. See https://github.com/jpaulm/javafbp/blob/master/src/main/java/com/jpaulmorrison/fbp/core/engine/OutputPort.java

On Wed, Nov 16, 2016 at 12:02 PM, Dan Rumney notifications@github.com wrote:

As I look at it, you should roll my work back... my implementation of connections is way off.

My ProcessConnection class has a single capacity, but multiple upstream processes... this is a totally incorrect representation of the way that multiple OutputPorts can fan into an InputPort.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261005604, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ8v7vC3l-JOe3GMoUlNUP2a5p-FXks5q-zc0gaJpZM4KvycM .

dancrumb commented 7 years ago

OK... so here's what to do:

git reset --soft <commit hash> git add . git commit -m "Rollback breaking changes" git push

That will create a new commit that rolls everything back, rather than destroying the shared history. That way, the changes are still viewable, but they're no longer at the front of the tree

dancrumb commented 7 years ago

Yes... the ProcessConnection should have capacity, but it should be between a single upstream process and a single downstream process. My implementation has it between multiple upstream processes and a single downstream process. This is incorrect, since, in the case where multiple OUT ports lead into an IN, there could be a different capacity for each connection in the FBP graph.

dancrumb commented 7 years ago

An example would be:

aplha OUT -> (5) IN gamma
beta OUT -> (10) IN gamma
delta OUT -> IN gamma
dancrumb commented 7 years ago

Also, the OutputPort does have a closed property; it's inherited from the Port class

jpaulm commented 7 years ago

In this case, jsfbp will differ from all the other implementations! We have always allowed multiple output ports (on different processes) to feed into a single Connection, but in-transit IPs are held in the Connection, not in the output ports - the IPs arriving at that connection are merged in a first-come, first-served order. If you don't do that, how would you decide in what order to read from the multiple outports... if you could, which you can't, as "receive" references a connection, not an output port.

Sorry, Dan!

On Wed, Nov 16, 2016 at 12:35 PM, Dan Rumney notifications@github.com wrote:

Yes... the ProcessConnection should have capacity, but it should be between a single upstream process and a single downstream process. My implementation has it between multiple upstream processes and a single downstream process. This is incorrect, since, in the case where multiple OUT ports lead into an IN, there could be a different capacity for each connection in the FBP graph.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261014982, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4iQkcbFkHBzYbM27PnJqWBg6e4gks5q-z77gaJpZM4KvycM .

jpaulm commented 7 years ago

"Finally, you can modify https://github.com/jpaulm/jsfbp/blob/master/test/ components/breader.js#L9 to read describe.skip to skip that test case. I think it's better to 'skip' than remove, since you have that reminder that you have a test that's not currently working without it breaking your workflow."

Do you leave the function, or remove the whole thing?

TIA

Paul

On Wed, Nov 16, 2016 at 12:00 PM, Dan Rumney notifications@github.com wrote:

Hi Paul,

Went away for the weekend and been catching up.

Regarding suspending the sender, I believe this remains in place in the ProcessConnection code, specifically at: https://github.com/jpaulm/ jsfbp/blob/master/core/ProcessConnection.js#L79

At this point, if the number of pending IPs is greater than the connection capacity, then the process is put into WAITING_TO_SEND status and is not requeued for the runtime; then, the process yields, thereby suspending it until it's added to the runtime queue again.

This check occurs in a permanent loop that isn't broken until the IP that is being 'sent' can be added to the connection.

The absence of tests around this behaviour is definitely an oversight and a problem. Perhaps the first step is to try and craft a test that passes when the behaviour is correct and otherwise fails.

Certainly Travis didn't catch this, but I think that's more a shortcoming of the test suite rather than an inherent shortcoming of CI.

I think an update to the README is probably a good idea. Alternatively, another approach might be to branch off the current master branch and then revert all of the changes I put in, with a view to being more circumspect about the changes.

Finally, you can modify https://github.com/jpaulm/jsfbp/blob/master/test/ components/breader.js#L9 to read describe.skip to skip that test case. I think it's better to 'skip' than remove, since you have that reminder that you have a test that's not currently working without it breaking your workflow.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261004937, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJxPqLAWGFARu3U3GLSnowyOAedGTks5q-zangaJpZM4KvycM .

dancrumb commented 7 years ago

So, in the case of the graph above, having a single capacity for a Connection object doesn't make sense, if that connection encapsulates all incoming connections to an IN port.

I can see how an implementation of a Connection could do this, but not with a single capacity value... there would need to be one for each incoming processes. Definitely doable, but not what appears to be in place in the Java implementation.

dancrumb commented 7 years ago

For skipping a test:

describe( /* everything */);

becomes

describe.skip( /* everything */);

jpaulm commented 7 years ago

B**dy GitHub!

I got to git push - but I had updated Readme, so it said to do a pull first. Did that, and got:

remote: Counting objects: 3, done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (3/3), done. From https://github.com/jpaulm/jsfbp 8432d81..89ae5d7 master -> origin/master Auto-merging core/trace.js CONFLICT (content): Merge conflict in core/trace.js Auto-merging core/runtimes/FiberRuntime/index.js CONFLICT (content): Merge conflict in core/runtimes/FiberRuntime/index.js Auto-merging core/ProcessConnection.js CONFLICT (content): Merge conflict in core/ProcessConnection.js Auto-merging core/Process.js CONFLICT (content): Merge conflict in core/Process.js Auto-merging core/OutputPort.js CONFLICT (content): Merge conflict in core/OutputPort.js Auto-merging core/Network.js CONFLICT (content): Merge conflict in core/Network.js Auto-merging core/InputPort.js CONFLICT (content): Merge conflict in core/InputPort.js Auto-merging core/IIPConnection.js CONFLICT (content): Merge conflict in core/IIPConnection.js CONFLICT (modify/delete): core/FIFO.js deleted in HEAD and modified in 89ae5d792 08faef88399b415e42fdb8dd8205ca5. Version 89ae5d79208faef88399b415e42fdb8dd8205ca 5 of core/FIFO.js left in tree. CONFLICT (modify/delete): core/Connection.js deleted in HEAD and modified in 89a e5d79208faef88399b415e42fdb8dd8205ca5. Version 89ae5d79208faef88399b415e42fdb8dd 8205ca5 of core/Connection.js left in tree. Auto-merging components/repl.js Auto-merging README.md Automatic merge failed; fix conflicts and then commit the result. C:\Users\Paul\Documents\GitHub\jsfbp [master ↕ +2 ~16 -2 !8 | +0 ~0 -0 !10 !]>

Try again?! Maybe if I did a reset --hard from the same commit hash...? Would that get rid of merge indicator?

(Sorry!)

Paul

On Wed, Nov 16, 2016 at 12:34 PM, Dan Rumney notifications@github.com wrote:

OK... so here's what to do:

git reset --soft git add . git commit -m "Rollback breaking changes" git push

That will create a new commit that rolls everything back, rather than destroying the shared history. That way, the changes are still viewable, but they're no longer at the front of the tree

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261014576, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ5IveEu51o8h-x6FNKOWfwZ_NlEyks5q-z6fgaJpZM4KvycM .

dancrumb commented 7 years ago

Git can be a pain when you're trying revert stuff :)

OK... so, first, make sure you have no changes that have not been committed (shouldn't be a problem).

1) Reset your local repo to match the remote one:

git fetch origin
git reset --hard origin/master

2) Soft reset to the commit you want to be at:

git reset --soft <commit hash>

3) Create a new commit to rollback to this point in time:

git add .
git commit -m "Rollback breaking changes"

4) Push the new state to Github:

git push
jpaulm commented 7 years ago

Close but no banana!

The final git push said

C:\Users\Paul\Documents\GitHub\jsfbp [master ↕]> git push To https://github.com/jpaulm/jsfbp.git ! [rejected] master -> master (non-fast-forward) error: failed to push some refs to 'https://github.com/jpaulm/jsfbp.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

Sorry, have to run out for a couple of hours - I feel we're nearly there!

On Wed, Nov 16, 2016 at 1:10 PM, Dan Rumney notifications@github.com wrote:

Git can be a pain when you're trying revert stuff :)

OK... so, first, make sure you have no changes that have not been committed (shouldn't be a problem).

  1. Reset your local repo to match the remote one:

git fetch origin git reset --hard origin/master

  1. Soft reset to the commit you want to be at:

git reset --soft

  1. Create a new commit to rollback to this point in time:

git add . git commit -m "Rollback breaking changes"

  1. Push the new state to Github:

git push

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261024702, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJx-Fw1mwtiR0eIpL6wp-JQXg8GgNks5q-0cigaJpZM4KvycM .

dancrumb commented 7 years ago

OK finally figured it out:

git reset --hard <your commit of choice>
git clean -df
git reset --soft @{1}
git commit -m "Rollback of breaking changes"
git push

I did this on my copy, ran npm install and npm test and everything (except breader) seemed to work.

The breader issue is a line-endings issues due to OS differences.

jpaulm commented 7 years ago

Thanks, Dan,

Will give that a try as soon as I get home!

Regards,

Paul

On Nov 16, 2016 2:06 PM, "Dan Rumney" notifications@github.com wrote:

OK finally figured it out:

git reset --hard git clean -df git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

I did this on my copy, ran npm install and npm test and everything (except breader) seemed to work.

The breader issue is a line-endings issues due to OS differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261040558, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4zO3efM4534-rzzio_IFH6vL8beks5q-1RMgaJpZM4KvycM .

jpaulm commented 7 years ago

Never saw git clean before! Interesting!

No errors...

Did next statement, and got

C:\Users\Paul\Documents\GitHub\jsfbp [master ↓]> git reset --soft @{1} At line:1 char:21

Huh?! Maybe my posh~git is different from yours...?

On Wed, Nov 16, 2016 at 2:06 PM, Dan Rumney notifications@github.com wrote:

OK finally figured it out:

git reset --hard git clean -df git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

I did this on my copy, ran npm install and npm test and everything (except breader) seemed to work.

The breader issue is a line-endings issues due to OS differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261040558, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4zO3efM4534-rzzio_IFH6vL8beks5q-1RMgaJpZM4KvycM .

dancrumb commented 7 years ago

Try ORIG_HEAD instead of @{1}

It could be that '@{1}' has some special meaning to your shell

On Wed, Nov 16, 2016 at 3:31 PM Paul Morrison notifications@github.com wrote:

Never saw git clean before! Interesting!

No errors...

Did next statement, and got

C:\Users\Paul\Documents\GitHub\jsfbp [master ↓]> git reset --soft @{1} At line:1 char:21

  • git reset --soft @{1}
  • ~ Missing '=' operator after key in hash literal. At line:1 char:21
  • git reset --soft @{1}
  • ~ The hash literal was incomplete.
  • CategoryInfo : ParserError: (:) [], ParentContainsErrorRecordEx ception
  • FullyQualifiedErrorId : MissingEqualsInHashLiteral

Huh?! Maybe my posh~git is different from yours...?

On Wed, Nov 16, 2016 at 2:06 PM, Dan Rumney notifications@github.com wrote:

OK finally figured it out:

git reset --hard git clean -df git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

I did this on my copy, ran npm install and npm test and everything (except breader) seemed to work.

The breader issue is a line-endings issues due to OS differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261040558, or mute the thread < https://github.com/notifications/unsubscribe-auth/AATGJ4zO3efM4534-rzzio_IFH6vL8beks5q-1RMgaJpZM4KvycM

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261078541, or mute the thread https://github.com/notifications/unsubscribe-auth/AANhT7bB1ltGJoMcF-eOR9CfCO3IJgeKks5q-3YugaJpZM4KvycM .

jpaulm commented 7 years ago

Dan, please ignore the previous note - I tried it in regular command prompt, and it worked... until the git push, which gave the same messages again as before:

error: failed to push some refs to 'https://github.com/jpaulm/jsfbp.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I assume this is because of the change I made to Readme - if so, my fault... but it should be possible to get around it, as I don't case if my recent change to Readme gets wiped.

On Wed, Nov 16, 2016 at 4:31 PM, Paul Morrison jpaulmorr@gmail.com wrote:

Never saw git clean before! Interesting!

No errors...

Did next statement, and got

C:\Users\Paul\Documents\GitHub\jsfbp [master ↓]> git reset --soft @{1} At line:1 char:21

  • git reset --soft @{1}
  • ~ Missing '=' operator after key in hash literal. At line:1 char:21
  • git reset --soft @{1}
  • ~ The hash literal was incomplete.
    • CategoryInfo : ParserError: (:) [], ParentContainsErrorRecordEx ception
    • FullyQualifiedErrorId : MissingEqualsInHashLiteral

Huh?! Maybe my posh~git is different from yours...?

On Wed, Nov 16, 2016 at 2:06 PM, Dan Rumney notifications@github.com wrote:

OK finally figured it out:

git reset --hard git clean -df git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

I did this on my copy, ran npm install and npm test and everything (except breader) seemed to work.

The breader issue is a line-endings issues due to OS differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261040558, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4zO3efM4534-rzzio_IFH6vL8beks5q-1RMgaJpZM4KvycM .

dancrumb commented 7 years ago

That's weird

If you're fine losing the README changes, this:

git reset --hard origin/master
git clean -df
git reset --hard <your commit of choice>
git reset --soft @{1}
git commit -m "Rollback of breaking changes"
git push
jpaulm commented 7 years ago

Took it! Phew! Thanks a million!

Onwards and upwards!

Paul

On Wed, Nov 16, 2016 at 4:52 PM, Dan Rumney notifications@github.com wrote:

That's weird

If you're fine losing the README changes, this:

git reset --hard origin/master git clean -df git reset --hard git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261084074, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4xR9sUMjwBkaglYu4uWQjmzkvXBks5q-3sxgaJpZM4KvycM .

jpaulm commented 7 years ago

Beautiful! Thanks! I guess these kinds of errors happen - I had a similar problem with DrawFBP - although it was spotted faster!

Regards,

Paul

On Wed, Nov 16, 2016 at 1:05 PM, Dan Rumney notifications@github.com wrote:

For skipping a test:

describe( /* everything */);

becomes

describe.skip( /* everything */);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261023225, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ31Ly6pdZQj2nAmp_xv8-76UpP-Eks5q-0XSgaJpZM4KvycM .

jpaulm commented 7 years ago

Sorry, Dan, had to backup a bit further - to 99DA196 - before FIFO.js and Connection.js were introduced. Still works, except that I have to skip breader again - will do that tomorrow.

Readme should probably be checked.

Thanks for your help,

Paul

On Wed, Nov 16, 2016 at 4:52 PM, Dan Rumney notifications@github.com wrote:

That's weird

If you're fine losing the README changes, this:

git reset --hard origin/master git clean -df git reset --hard git reset --soft @{1} git commit -m "Rollback of breaking changes" git push

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jpaulm/jsfbp/issues/78#issuecomment-261084074, or mute the thread https://github.com/notifications/unsubscribe-auth/AATGJ4xR9sUMjwBkaglYu4uWQjmzkvXBks5q-3sxgaJpZM4KvycM .