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.

Sign up for GitHub

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

Jump to bottom

MAKEFILE: Auto-generate dependencies for scummvm.rc #2921

Merged
merged 1 commit into from
Apr 23, 2021
Merged

MAKEFILE: Auto-generate dependencies for scummvm.rc #2921

merged 1 commit into from
Apr 23, 2021

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Apr 6, 2021 •
edited
Loading

No description provided.

@orgads
Copy link
Contributor Author

orgads commented Apr 6, 2021

Missing: $(srcdir)/ prefix for most deps, glue for actually using the d file, and removal of the existing deps in win32.mk.

Pro: Auto-detected
Con: Lists more than actually used. For example, if an engine is excluded, the rc file will still depend on its resources.

@orgads orgads mentioned this pull request Apr 6, 2021
Merged
@lephilousophe
Copy link
Member

I think this PR is better than #2919

digitall
digitall reviewed Apr 6, 2021
View reviewed changes
Makefile.common Outdated Show resolved Hide resolved
@orgads orgads changed the title WIP: MAKEFILE: Add dependencies for scummvm.rc MAKEFILE: Auto-generate dependencies for scummvm.rc Apr 6, 2021
@orgads
Copy link
Contributor Author

orgads commented Apr 6, 2021

Ready for review.

@orgads
Copy link
Contributor Author

orgads commented Apr 7, 2021

Don't merge yet. I forgot config.mk.

@orgads
Copy link
Contributor Author

orgads commented Apr 7, 2021

Done. Ready for review.

@orgads
Copy link
Contributor Author

orgads commented Apr 9, 2021

@digitall?

@digitall
Copy link
Member

digitall commented Apr 9, 2021

Looks reasonable. The comments you added to explain the sed magic look fine. Have not tested here as not on Windows, maybe @ccawley2011 should review?

ccawley2011
ccawley2011 reviewed Apr 9, 2021
View reviewed changes
Copy link
Member

@ccawley2011 ccawley2011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright, just one minor comment.

Makefile.common Outdated Show resolved Hide resolved
@orgads
Copy link
Contributor Author

orgads commented Apr 22, 2021

ping?

@sev-
Copy link
Member

sev- commented Apr 23, 2021

Thanks, makes sense. Good work. Merging.

@sev- sev- merged commit 59752c2 into scummvm:master Apr 23, 2021
@orgads orgads deleted the rc-deps-2 branch April 23, 2021 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digitall digitall digitall left review comments

@ccawley2011 ccawley2011 ccawley2011 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@orgads @lephilousophe @digitall @sev- @ccawley2011