Hi Marcel,


I think I preserved the original message - I only removed the little word "inbox" for the sake of abstraction. Is this what you are referring to?


> This looks like "premature abstraction".


Hm ... Can't we call it a feature of the Monticello API instead? I just wanted to get rid of these "magic numbers" in the UI methods. Smalltalk idioms told me to extract them into explaining methods. :-)


Best,

Christoph

<http://www.hpi.de/>
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Donnerstag, 11. März 2021 08:58:43
An: squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Monticello-ct.737.mcz

-1

#save does not inform the user about the actual problem between Inbox and Trunk/Treated anymore. Good error messages are paramount for usability.

To make this a constructive discussion: This looks like "premature abstraction". What are other examples for source/sink repositories?

Best,
Marcel

Am 09.03.2021 14:00:07 schrieb commits at source.squeak.org <commits at source.squeak.org>:

A new version of Monticello was added to project The Inbox:
http://source.squeak.org/inbox/Monticello-ct.737.mcz

==================== Summary ====================

Name: Monticello-ct.737
Author: ct
Time: 9 March 2021, 1:59:56.826559 pm
UUID: 697dc3bc-f4f7-3e43-b05d-d6a1f5fb421e
Ancestors: Monticello-mt.736

Revises warnings shown before saving a version. In #checkForNewVersions, do not only check the current repository (e.g. inbox) but also the sink repository (e.g. trunk). Also generalizes the idea of sink repositories and resolve hard-coded repositores in Monticello-UI.

=============== Diff against Monticello-mt.736 ===============

Item was added:
+ ----- Method: MCRepository>>allSinkRepositories (in category 'accessing') -----
+ allSinkRepositories
+ "If the workflow of the receiver requires to move new versions into one of any other repositories in the end, answer all these available sink repositories here."
+
+ self = MCRepository inbox ifTrue: [
+ ^ {MCRepository trunk. MCRepository treated}].
+
+ ^ Array empty!

Item was added:
+ ----- Method: MCRepository>>defaultSinkRepository (in category 'accessing') -----
+ defaultSinkRepository
+ "If the workflow of the receiver requires to move new versions into another repository in the end, answer this default sink repository here."
+
+ self = MCRepository inbox ifTrue: [
+ ^ MCRepository trunk].
+
+ ^ nil!

Item was changed:
----- Method: MCRepositoryInspector>>save (in category 'actions') -----
save
+
+ | targetRepository |
+ targetRepository := self pickRepository ifNil: [^ self].
+
+ (self repository allSinkRepositories includes: targetRepository) ifTrue: [
+ self notify: (String streamContents: [:stream |
+ stream nextPutAll: 'Versions from this repository should only be moved, not copied.' translated.
+ (self repository isKindOf: MCHttpRepository) ifTrue: [
+ stream space; nextPutAll: ('Instead, use the web interface via {1} to manage contributions.' translated format: {(Url absoluteFromText: self repository description)
+ path: '';
+ downloadUrl})].
+ stream cr; cr; nextPutAll: 'Do you want to proceed anyway?' withCRs translated])].
+
+ repository storeVersion: self version.!
- self pickRepository ifNotNil:
- [:repository |
- (self repository = MCRepository inbox and:
- [repository = MCRepository trunk or: [repository = MCRepository treated]]) ifTrue:
- [self notify: 'Versions from the inbox should only be moved, not copied. Instead, use the web interface via source.squeak.org to manage inbox contributions.\\Do you want to proceed anyway?' translated withCRs].
- repository storeVersion: self version]!

Item was changed:
----- Method: MCWorkingCopyBrowser>>checkForNewerVersions (in category 'actions') -----
checkForNewerVersions
+
| newer |
newer := workingCopy possiblyNewerVersionsIn: self repository.
+ (newer notEmpty ==> [
+ self confirm: ('{1}\\{2}\\{3}' withCRs translated format: {
+ 'CAUTION!! These versions in the repository may be newer:' translated.
+ (newer asCommaString withNoLineLongerThan: 150) truncateWithElipsisTo: 5 * 149.
+ 'Do you really want to save this version?' translated}) ])
+ ifFalse: [^ false].
+
+ self repository defaultSinkRepository ifNotNil: [:sinkRepository |
+ newer := workingCopy possiblyNewerVersionsIn: sinkRepository.
+ (newer notEmpty ==> [
+ self confirm: ('{1}\\{2}\\{3}' withCRs translated format: {
+ 'CAUTION!! These versions in the sink repository\({1})\may be newer:' withCRs translated format: {sinkRepository description}.
+ (newer asCommaString withNoLineLongerThan: 150) truncateWithElipsisTo: 5 * 149.
+ 'Do you really want to save this version?' translated}) ])
+ ifFalse: [^ false]].
+
+ ^ true!
- ^ newer isEmpty or: [
- self confirm: 'CAUTION!! These versions in the repository may be newer:',
- String cr, ((newer asCommaString withNoLineLongerThan: 150) truncateWithElipsisTo: 5 * 149), String cr,
- 'Do you really want to save this version?'].!


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210311/a50bfc7a/attachment.html>