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

Rebase over SDCC r11930 #16

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Rebase over SDCC r11930 #16

wants to merge 6 commits into from

Conversation

pixelherodev
Copy link

No description provided.

@pixelherodev
Copy link
Author

cc @aviallon @MaxLeiter

@pixelherodev
Copy link
Author

@aviallon For future reference: the process I used to import changes relatively cleanly:

  • Format all the codes. clang-format -i --style=file src/*.{c,h,hpp,cc} src/backend/*.{c,cc,h} src/util/*.{c,h}

A simpler formatter would be nice, but I used what I had on hand

  • Commit that and leave it on master. For the future, we'll want to enforce this for every commit, so it doesn't need to be done in bulk.

  • Branch.

  • Copy all upstream files in place.

    • rsync ../../../archives/sdcc/sdcc/src/SDCC* src/
    • rsync ../../../archives/sdcc/sdcc/src/z80/*.{h,c,cc,def,i} src/backend/
    • etc
  • Selectively apply patches with git add -p. It's been five years, and there's about 10KLoC changes. If we do this regularly (monthly, maybe?), we should be able to easily apply only those patches we want and reject the ones we don't. Alternatively, we can go over the revisions upstream now that there's fewer to look at, and apply those instead. This time, I just took my time and combed over the changes, adding them piecemeal.

  • Once all desired changes are selected, make a temporary commit.

  • Remove unwanted changes with e.g. git reset --hard.

  • Test.

  • Add fixes in their own commits, with careless messages :p

  • Rebase, merge together commits which make sense as one, clean up messages. Rule of thumb: every single commit should build without issues. If any don't, that's a problem, and it should be fixed in this step.

  • Review

  • PR

@pixelherodev
Copy link
Author

Ignoring the changelog, this is +16328 -9249, which should be much easier to review than the last attempt, but still might be better split into pieces.

@aviallon
Copy link
Member

This is very neat ! Thanks !

@aviallon
Copy link
Member

I'm happy you find the time to work on this... Do you think that this new codebase would support c99 ? (At least variable declarations not at the beginning of a context)
This would be cool 😃

@pixelherodev
Copy link
Author

pixelherodev commented Oct 30, 2020

It already did! :P

It supports more of C99 now, but it already supported a good chunk with --std-c99.

@aviallon
Copy link
Member

Unfortunately, it didn't for this particular feature.

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.

2 participants