-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
package compatibility fallout from recent changes #121
Comments
ping @mikesperber |
@samth ProfJ was broken before, for other reasons, and we decided to leave it. I've sent fixes to CSC104. |
Are these files/functions hard to support in the new regime? All of these packages compiled successfully before, and I'd prefer not to break things if we can just keep them alive. |
@samth Mike sent a patch to our friends in Toronto. @mikesperber Did you ever hear back whether the patch was okay? |
I haven't had a chance to try it but am fine with csc104 breaking on snapshots until I get a chance over the holidays to update it (there's also a rewrite of the package in progress anyway). |
I appreciate making an effort to move people to new APIs, but unless these old APIs are very hard to support, the fact that they were used somewhere suggests that they might be used elsewhere too, and we should avoid breaking them. |
@samth Mike, Robby, Matthew and I discussed this change extensively and we brought it up during a Tuesday meeting. We all thought it meant progress. |
Obviously the change overall is good. But we should figure out if we can address the particular compatibility issues that we've discovered here without doing too much extra work. |
The APIs that affected ProfessorJ and CSC104 would have been impossible to support while still making the improvements the test-engine rewrite made. I did check. Note that there are many other packages that also use |
I've made a PR for profj that makes it compile again. |
The following packages fail to build due to test-engine changes:
builder
fromtest-engine/racket-tests
)test-engine/test-display.scm
)profj
)The text was updated successfully, but these errors were encountered: