-
Notifications
You must be signed in to change notification settings - Fork 106
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
Makefile: target to import all programs from Optimism #2906
Conversation
23654a2
to
b8a6a11
Compare
b8a6a11
to
49235d3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2906 +/- ##
==========================================
- Coverage 73.45% 73.45% -0.01%
==========================================
Files 259 259
Lines 61593 61593
==========================================
- Hits 45244 45243 -1
- Misses 16349 16350 +1 ☔ View full report in Codecov by Sentry. |
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.
This is super useful! My only request is that the linked files shouldn't be committed, i.e. the entire resources/programs/{mips,riscv}/bin
directory should be ignored (not just the object files). It seems like this was also your intention since make clean
blows this directory away.
The only issue I see by not committing them is that the CI (and users) will have to build them, and therefore would need to add the mips toolchain on their machine. |
That's a fair point that it's annoying to require it for devs, it just seems weird that running BTW I can confirm that these are working with #2911 . |
@dannywillems I would suggest that we use a github workflow with a I implemented this strategy in #2911. You can see what this workflow looks like here, and here is an example upload |
49235d3
to
4cf77b5
Compare
4cf77b5
to
2637499
Compare
Import all MIPS programs from Optimism, and tweak it to make it compatible with our ELF loader. Requested by @martyall to have more small test programs for the o1vm MIPS edition.