Skip to content

build: Remove unused RES_IMAGES #19394

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
Merged
merged 1 commit into from
Jun 29, 2020
Merged

build: Remove unused RES_IMAGES #19394

merged 1 commit into from
Jun 29, 2020

Conversation

Bushstar
Copy link
Contributor

Remove RES_IMAGES. Seems to be unused since 2015 in the commit below.

98c222b#diff-9a4f3a253de77bf90b107bdf5283ebc3R317

The src/qt/res/images to which it was used with is no longer present either.

@fanquake fanquake changed the title [build] Remove unused RES_IMAGES build: Remove unused RES_IMAGES Jun 29, 2020
@fanquake
Copy link
Member

Concept ACK. There another instance of $(RES_IMAGES) here:

bitcoin/src/Makefile.qt.include

Line 293 in d3a5dbf

$(QT_QRC) $(QT_QRC_LOCALE) $(QT_TS) $(RES_ICONS) $(RES_IMAGES) $(RES_MOVIES)

@Bushstar Bushstar force-pushed the bitcoin-remove-res-images branch from 4ffd304 to 53361dd Compare June 29, 2020 04:49
@Bushstar
Copy link
Contributor Author

@fanquake PR updated, thanks.

@hebasto
Copy link
Member

hebasto commented Jun 29, 2020

Concept ACK.

hebasto
hebasto approved these changes Jun 29, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 53361dd, tested on Linux Mint 20 (x86_64).

@maflcko maflcko merged commit 748178f into bitcoin:master Jun 29, 2020
@thijstriemstra
Copy link

What about $(RES_MOVIES)? Are there really video clips included..

@hebasto
Copy link
Member

hebasto commented Jun 29, 2020

What about $(RES_MOVIES)? Are there really video clips included..

bitcoin/src/Makefile.qt.include

Line 279 in 1269cab

RES_MOVIES = $(wildcard $(srcdir)/qt/res/movies/spinner-*.png)

@maflcko
Copy link
Member

maflcko commented Jun 29, 2020

RES_ANIMATION would be a better name for the spinner, I guess

@thijstriemstra
Copy link

thijstriemstra commented Jun 29, 2020 •
edited
Loading

Renaming the directory would also help I guess: $(srcdir)/qt/res/movies What does 'res' even mean, resources? Maybe also use resources instead of res?

@Bushstar
Copy link
Contributor Author

Created a PR here to rename movies to animation.

#19414

Movies has always seemed like an odd term for the spinner animation.

@hebasto hebasto mentioned this pull request Jun 29, 2020
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hebasto hebasto hebasto approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@Bushstar @fanquake @hebasto @thijstriemstra @maflcko @DrahtBot