Provide error information directly when class definition loading fails

[dolezvo1]

Fixes Metacello/metacello#559

[welcome]

Thanks for opening this pull request! Now continious integration (CI) will build Pharo with your change and run all tests. This might fail due to many reasons! Please check if your PR breaks the build or makes tests fail. Feel free to add comments to the PR. After this, before your PR can be merged it needs one or more reviews. Do not hesitate to ask people (on the Mailinglist or Discord) to help! When the CI shows no problems and there are positive reviews, your PR will be merged.

[dolezvo1]

I think the original code might be wrong in a slight way, as indicated by the note that a second pass might fix the errors. Perhaps this would be fixed if the terminating condition in loadClassDefinitions checked equivalence of the collected errors as opposed to only checking the number of the collected errors?

[Ducasse]

Please notice that I'm not sure that Pharo use the Metacello repository

[jecisc]

I changed this code in P12. I don't have time to review now but I'll review it next week

[Ducasse]

Tx Cyril :)

[jecisc]

I think the original code might be wrong in a slight way, as indicated by the note that a second pass might fix the errors. Perhaps this would be fixed if the terminating condition in loadClassDefinitions checked equivalence of the collected errors as opposed to only checking the number of the collected errors?

Checking the size should be enough because we will not load definitions in the n+1 pass that was not already present in the n pass of the loading. So we don't need to check for equivalences

[jecisc]

I just have one little code cleaning I suggested, except that, the changes seems good to me

[jecisc]

Suggested change

	errorsDict keysDo: [ :ea |
	s
	space;
	space;
	nextPutAll: ea summary;
	nextPutAll: ' (';
	nextPutAll: ((errorsDict at: ea) description asString);
	nextPutAll: ')';
	errorsDict keysAndValuesDo: [ :definition :error |
	s
	space;
	space;
	nextPutAll: definition summary;
	nextPutAll: ' (';
	nextPutAll: error description asString;
	nextPutAll: ')';

[Ducasse]

@jecisc may be we should do your changes and integrate this PR

[jecisc]

I don't have the rights to push my changes to this PR. I'll just integrate it. My changes were for readability only

[welcome]

Congrats on merging your first pull request! Do another one! We try to have a list of (relatively) easy issues here: https://github.com/orgs/pharo-project/projects/8.

GitHub

Build software better, together

GitHub is where people build software. More than 100 million people use GitHub to discover, fork, and contribute to over 420 million projects.