search

pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.19k stars 353 forks source link

The default operation of MCMergeOrLoadWarning should be to perform the load, not cancel the operation #2233

Open theseion opened 5 years ago

theseion commented 5 years ago

Migrated from https://pharo.fogbugz.com/f/cases/19280/The-default-operation-of-MCMergeOrLoadWarning-should-be-to-perform-the-load-not-cancel-the-operation //-----

The default action for MCMergeOrLoadWarning is to cancel the operation (nil). true will force the load and false will produce a merge.

I think MCMergeOrLoadWarning should be changed, so that the default is to force the load.

A search across issues shows that MCMergeOrLoadWarning is a recurring problem when loading code. Metacello has worked around the issue. In Squeak this problem does not exist.

See also this discussion to understand why this is a problem in particular for automated builds with TravisCI (SmalltalkCI).

stale[bot] commented 5 years ago

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 8 months. It will be closed in 1 month if no further activity occurs. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

Joel on Software
Software Inventory
Imagine, for a moment, that you came upon a bread factory for the first time. At first it just looks like a jumble of incomprehensible machinery with a few people buzzing around. As your eyes adjus…
theseion commented 5 years ago

Still relevant.

stale[bot] commented 4 years ago

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

theseion commented 4 years ago

Appears to be fixed in Pharo 8, but needs to be checked.

guillep commented 4 years ago

Pharo 9.0 commit 177a16763fb410ad36cc119cb499b004a3622536.

I think the issue is still there:

defaultAction
    ^ ( UIManager default
        confirm: self messageText
        trueChoice: 'Load'
        falseChoice: 'Merge'
        cancelChoice: 'Cancel'
        default: nil ).

Fixing this would be changing the default value to true?

theseion commented 4 years ago

Well, yes, you're right, it's not "fixed" but all users in the image handle the exception gracefully. The real fix would be to change the default to true, as in Squeak. That may break other handling code though.