Find/Replace dialog: remove ring-tone #1908 #1923
Conversation
Test Results 1 812 files ±0 1 812 suites ±0 1h 39m 35s ⏱️ + 7m 0s For more details on these failures, see this check. Results for commit b640bb3. ± Comparison against base commit 64cf545. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
I absolutely like the proposal to remove the beep functionality (and I think particularly @vogella will do as well 🙂).
Just two comments on the proposed changes:
- The removal seems incomplete to me, as the
allowBeepparameter inevaluateFindReplaceStatusis now unused. In my opinion, this should be cleaned up throughout the call hierarchy. - This change does not seem to depend on [Modern Find/Replace] Implemented Find-Replace Overlay #1192 but currently contains the commit of that PR. You could rebase the single commit relevant for this PR onto master instead of the other PR.
Wittmaxi
force-pushed
the
MW_remove_dialog_ringtone
branch
2 times, most recently
from
2e6a42d
to
d749474
Compare
|
@HeikoKlare I believe that the issues you just mentionned are addressed |
HeikoKlare
force-pushed
the
MW_remove_dialog_ringtone
branch
from
d749474
to
1c465fd
Compare
There was a problem hiding this comment.
Looks good now, thank you! I have added a version bump for 4.33 stream to see if builds run fine. In case this is merged after some other PR affecting the same project (like #1192), the version bump will be automatically removed via rebase.
Removes the beep which is issued after some unsuccessful operations in the find/replace dialog. fixes eclipse-platform#1908
HeikoKlare
force-pushed
the
MW_remove_dialog_ringtone
branch
from
1c465fd
to
b640bb3
Compare
|
Failing test is unrelated and already documented: #294 |
Removes the beep which is issued after some unsuccessful operations in
the find/replace dialog.
fixes #1908