-
Notifications
You must be signed in to change notification settings - Fork 58
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
embed readmefs in all sims #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good overall! I left a few minor comments. Also, you should update the versions of emergent and Cogent Core used after emer/emergent#134 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angelajt Thanks for fixing most of those issues.
I also just noticed that the Objrec binary got added in one of your commits (your first one new to this PR), which would increase the size of the repository. If you could please carefully run the commands below one at a time to overwrite the history to fix that, that would be great (run these on your branch as it is now with no new commits and no uncommitted changes). You should make a local copy of your folder first as a backup.
git reset --soft HEAD~4
rm ch6/objrec/Objrec
git commit -am "embed readme in all sims"
git push -f
Please be careful with your git add
going forward. Everything should be fine after you run those commands, so no worries. Please let me know if there are any issues.
Oops, sorry about that! I removed most of the binaries before committing but I guess I was in a hurry and that one slipped past my radar. I'll take care of that and the commented code when I have the chance. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angelajt Thanks for your fixes. This looks all good to merge once the emergent PR is done.
Note: have not tested on web because I'm on Linux