Skip to content
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

Add MS-DOS platform and Watcom compilers. #794

Merged
merged 5 commits into from
Jul 3, 2023
Merged

Conversation

OmniBlade
Copy link
Contributor

Provisional implementation, need some clarification on where the tools should actually go and if they should be pulled down in downloads.py as I currently have it or in the Dockerfile for the backend. Also, the MS-DOS icon isn't scaling right for me and I can't figure out how to fix it.

@ethteck
Copy link
Member

ethteck commented Jun 27, 2023

if they should be pulled down in downloads.py

They indeed should, and it looks like you've set it up that way.

Can you explain what you mean by the logo not scaling right? I'll try to test locally soon.

Also, be sure to see the black failures / run black locally if you want it to auto-fix the formatting issues.

@ethteck
Copy link
Member

ethteck commented Jun 27, 2023

there are a bunch of (all?) compilers failing in the non-docker test, and 3 failing in the docker test. I'm not even sure if all 3 are new compilers either. very weird

backend/Dockerfile Outdated Show resolved Hide resolved
backend/coreapp/compilers.py Outdated Show resolved Hide resolved
backend/coreapp/platforms.py Outdated Show resolved Hide resolved
@mkst
Copy link
Collaborator

mkst commented Jun 27, 2023

This PR closes #468

@OmniBlade
Copy link
Contributor Author

if they should be pulled down in downloads.py

They indeed should, and it looks like you've set it up that way.

Can you explain what you mean by the logo not scaling right? I'll try to test locally soon.

Also, be sure to see the black failures / run black locally if you want it to auto-fix the formatting issues.

I think I've addressed the formatting and the comments relating to hard coded paths. I would like someone to verify the icon works correctly for them though before I take this out of draft. The issue I see is that it only displays the top left corner of the icon like its really zoomed in, but I thought I'd fixed that by adjusting the scaling in inkscape.

@mkst
Copy link
Collaborator

mkst commented Jun 28, 2023

The logo looks OK to me:
image

The --reloc option seems to generate a lot of noise for the i686
objdump.
@ethteck
Copy link
Member

ethteck commented Jul 1, 2023

@OmniBlade lgtm, but black has a couple more issues to resolve. Also, if you could refrain from force-pushing, it helps reviewers keep track of new changes over time

@ethteck
Copy link
Member

ethteck commented Jul 1, 2023

weird..other simple tests failed too in the most recent CI build. I'll look into it after the black things are fixed

@ethteck
Copy link
Member

ethteck commented Jul 3, 2023

@OmniBlade I think the same issues still remain, or at least the same lines

@ethteck ethteck merged commit fb34dbf into decompme:main Jul 3, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants