Skip to content

Conversation

@nephros
Copy link
Contributor

@nephros nephros commented May 28, 2024

Turns out these methods are not so dummy after all. (See #452)

One is a Signal, which shouldn't have an implementation at all.

The other is indeed dead code, but lack of usage o its parameter broke the
build. Instad of wrapping it in Q_UNUSED, lets just not have an
implementation, just the qdoc comment.

This now does silence qdoc, and does not break the build.

@Olf0
Copy link
Contributor

Olf0 commented May 29, 2024

Will look at this, try to comprehend the storyline which developed over years and approve, soon™.

@nephros
Copy link
Contributor Author

nephros commented Nov 6, 2024

Hey @Olf0 if you have the time it would be nice if you could approve and merge this.

The issue currently prevents building master on OBS.

Copy link
Contributor

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

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

IMO this confirms to stay away from any "easter shit" in the first place: It is additional, superfluous code, which causes a maintenance burden. 😛
Well, never mind my ranting.

Seriously:
I do not fully comprehend why you have taken out the textual pointers to the function declarations in the corresponding header files and to the locations these functions are used, which were intended not to loose track of these places which will have to be tackled when fully eliminating this unnecessary code. Knowing GitHub-search better now, I know they are easy to find again, but still someone will have to perform this (re)search again, then.

Still I approve this PR as I do not have an overly strong opinion about this, but I would be glad if you reconsider keeping the detailed comments in or explicitly name a reason why you believe they must go.

@nephros nephros merged commit 73c9a64 into sailfishos-patches:master Nov 15, 2024
1 check passed
@nephros nephros deleted the 452plus branch November 15, 2024 07:10
@nephros nephros restored the 452plus branch November 26, 2024 09:05
@nephros nephros deleted the 452plus branch November 26, 2024 10:41
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