pharo-vcs / iceberg

Iceberg is the main toolset for handling VCS in Pharo.
MIT License
134 stars 85 forks source link

Add Alerts to Iceberg's IceTipNewRepositoryPanel to Inform User of Incomplete Information #1834

Closed acyed closed 3 months ago

acyed commented 3 months ago

Fixes #16870

I have enhanced the validate methods by adding alerts to inform users about the necessary actions to continue. To allow the dialog to remain open, I had to modify the caller of validate.

This replaces the behavior of opening a debugging window on the failure of the assert calls.

Issue: https://github.com/pharo-project/pharo/issues/16870

Ducasse commented 3 months ago

Thanks a lot! You can really improve Pharo. We are so concentrated so time consuming tasks that we forget the simple changes that can improve the user experience. This is why we started to introduce papercut tags.

Ducasse commented 3 months ago

The broken tests seem unrelated

ReleaseTest
 ✗ #testUndeclared (149ms)
TestFailure: Found undeclared Variables: 
StSystemReporter in: SmalltalkCIPharo9>>#imageInfo
Gofer in: SCIGoferLoadSpec>>#basicLoadProjectOn:
ReleaseTest(TestAsserter)>>assert:description:resumable:
ReleaseTest(TestAsserter)>>assert:description:
ReleaseTest>>testUndeclared ...assert: remaining isEmpty description: description
ReleaseTest(TestCase)>>performTest
 ✗ #testNoEquivalentSuperclassMethods (1344ms)
TestFailure: Assertion failed
ReleaseTest(TestAsserter)>>assert:description:resumable:
ReleaseTest(TestAsserter)>>assert:description:
ReleaseTest(Object)>>assert:
ReleaseTest>>testNoEquivalentSuperclassMethods ...assert: methods size <= 326
ReleaseTest(TestCase)>>performTest
Ducasse commented 3 months ago

@jecisc @estebanlm can you have a look when you have time? This looks ok for me.

Rinzwind commented 3 months ago

This pull request seems to have introduced a bug, a NonBooleanReceiver is signaled when cloning a repository:

Ducasse commented 3 months ago

THanks Kris so I will revert it.