Skip to content

dtk dol apply: skip updating anonymous symbols by default #97

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

Merged
merged 2 commits into from
May 8, 2025

Conversation

robojumper
Copy link
Contributor

@robojumper robojumper commented Apr 28, 2025

In testing it reduces the diff noise for Skyward Sword when running ninja apply.

Also verified this by running ninja apply with this build for Twilight Princess, no unexpected @ or $ symbol name changes there.

@encounter
Copy link
Owner

Thoughts on making this the default behavior? (I haven't thought through it fully, so just curious)

@robojumper
Copy link
Contributor Author

Thoughts on making this the default behavior? (I haven't thought through it fully, so just curious)

I like the idea. I guess that way ninja apply in a dtk-template based project would do the less noisy, less merge-conflict-prone thing by default and people who need a "full" apply for some reason can run dtk dol apply manually or locally edit project.py for this hopefully rare situation where a full apply is needed.

@encounter
Copy link
Owner

In that case, maybe we can invert the flag? We can make it an optional --full or --replace flag.

@robojumper robojumper changed the title Add --relaxed flag to dtk dol apply to skip updating anonymous symbols dtk dol apply: skip updating anonymous symbols by default May 7, 2025
@encounter
Copy link
Owner

Thank you!

@encounter encounter merged commit 97302e5 into encounter:main May 8, 2025
16 checks passed
@robojumper robojumper deleted the dtk-dol-apply-relaxed branch May 8, 2025 07:02
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.

None yet

2 participants