The Trunk: System-cmm.913.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: System-cmm.913.mcz

commits-2
Chris Muller uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-cmm.913.mcz

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

Name: System-cmm.913
Author: cmm
Time: 31 August 2016, 6:32:49.269005 pm
UUID: 5a79d2c3-b230-4998-a790-612dc6a3471c
Ancestors: System-tfel.912

- Don't use colored parentheses and brackets in the Community dark theme, and intensify #dbRed a bit, it was too washed out.
- A new hook in Smalltalk run: to allow temporarily patched production systems running to be re-patched in the event of a restart.

=============== Diff against System-tfel.912 ===============

Item was changed:
  ----- Method: CommunityTheme class>>addDarkSyntaxHighlighting: (in category 'instance creation') -----
  addDarkSyntaxHighlighting: aUserInterfaceTheme
  "self createDark apply."
  | normal bold italic underlined darkMap | normal  := TextEmphasis normal.  bold:=TextEmphasis bold.  italic:=TextEmphasis italic.  underlined := TextEmphasis underlined.  darkMap := StrikeFont familyName: 'Darkmap DejaVu Sans' pointSize: 9.
  aUserInterfaceTheme
  set: #color for: #TextAction to: self dbBlue;
 
  set: #default for: #SHTextStylerST80 to: {self dbForeground};
  set: #invalid for: #SHTextStylerST80 to: {self dbInvalid};
  set: #excessCode for: #SHTextStylerST80 to: {self dbInvalid twiceDarker};
  "Descriptive text for humans, italicized."
  set: #comment for: #SHTextStylerST80 to: {self dbComment. italic};
  set: #unfinishedComment for: #SHTextStylerST80 to: {self dbComment darker. italic};
  set: #'$' for: #SHTextStylerST80 to: {self dbConstant};
  set: #character for: #SHTextStylerST80 to: {self dbConstant};
  set: #integer for: #SHTextStylerST80 to: {self dbConstant};
  set: #number for: #SHTextStylerST80 to: {self dbConstant};
  set: #- for: #SHTextStylerST80 to: {self dbForeground. bold};
  set: #= for: #SHTextStylerST80 to: {self dbForeground. bold};
  set: #symbol for: #SHTextStylerST80 to: {self dbBedrock};
  set: #stringSymbol for: #SHTextStylerST80 to: {self dbBedrock};
  set: #literalArray for: #SHTextStylerST80 to: {self dbForeground};
  set: #string for: #SHTextStylerST80 to: {self dbConstant};
  set: #unfinishedString for: #SHTextStylerST80 to: {self dbConstant darker};
  set: #assignment for: #SHTextStylerST80 to: {nil. bold};
  set: #ansiAssignment for: #SHTextStylerST80 to: {nil. bold};
  set: #literal for: #SHTextStylerST80 to: {nil. bold};
  set: #keyword for: #SHTextStylerST80 to: {self dbMessage};
  set: #binary for: #SHTextStylerST80 to: {self dbForeground. bold};
  set: #unary for: #SHTextStylerST80 to: {self dbMessage};
  set: #incompleteKeyword for: #SHTextStylerST80 to: {self dbMessage darker. {underlined. bold}};
  set: #incompleteBinary for: #SHTextStylerST80 to: {self dbMessage darker. underlined};
  set: #incompleteUnary for: #SHTextStylerST80 to: {self dbMessage darker. underlined};
  set: #undefinedKeyword for: #SHTextStylerST80 to: {self dbInvalid};
  set: #undefinedBinary for: #SHTextStylerST80 to: {self dbInvalid};
  set: #undefinedUnary for: #SHTextStylerST80 to: {self dbInvalid};
  "Delineate the selector (good for new users), and make the method look like a mini-document with a title."
  set: #patternKeyword for: #SHTextStylerST80 to: {self dbMessage lighter.  {bold. underlined}};
  set: #patternBinary for: #SHTextStylerST80 to: {nil. bold};
  set: #patternUnary for: #SHTextStylerST80 to: {self dbMessage lighter.  {bold. underlined}};
  set: #self for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #super for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #true for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #false for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #nil for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #thisContext for: #SHTextStylerST80 to: {self dbBedrock. bold};
  set: #return for: #SHTextStylerST80 to: {self dbForeground. bold};
  set: #patternArg for: #SHTextStylerST80 to: {self dbSelection twiceLighter. TextEmphasis normal. "darkMap"};
  set: #methodArg for: #SHTextStylerST80 to: {self dbSelection twiceLighter. TextEmphasis normal. "darkMap"};
  set: #blockPatternArg for: #SHTextStylerST80 to: {self dbSelection twiceLighter};
  set: #blockArg for: #SHTextStylerST80 to: {self dbSelection twiceLighter};
  set: #argument for: #SHTextStylerST80 to: {self dbSelection twiceLighter};
  set: #blockArgColon for: #SHTextStylerST80 to: {self dbBedrock};
+ set: #leftParenthesis for: #SHTextStylerST80 to: {self dbBedrock muchLighter};
+ set: #rightParenthesis for: #SHTextStylerST80 to: {self dbBedrock muchLighter};
+ set: #leftParenthesis1 for: #SHTextStylerST80 to: {self dbBedrock twiceLighter};
+ set: #rightParenthesis1 for: #SHTextStylerST80 to: {self dbBedrock twiceLighter};
+ set: #leftParenthesis2 for: #SHTextStylerST80 to: {self dbBedrock};
+ set: #rightParenthesis2 for: #SHTextStylerST80 to: {self dbBedrock};
+ set: #leftParenthesis3 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #rightParenthesis3 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #leftParenthesis4 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #rightParenthesis4 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #leftParenthesis5 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #rightParenthesis5 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #leftParenthesis6 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #rightParenthesis6 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #leftParenthesis7 for: #SHTextStylerST80 to: {Color yellow};
+ set: #rightParenthesis7 for: #SHTextStylerST80 to: {Color yellow};
+ set: #blockStart for: #SHTextStylerST80 to: {self dbBedrock muchLighter};
+ set: #blockEnd for: #SHTextStylerST80 to: {self dbBedrock muchLighter};
+ set: #blockStart1 for: #SHTextStylerST80 to: {self dbBedrock twiceLighter};
+ set: #blockEnd1 for: #SHTextStylerST80 to: {self dbBedrock twiceLighter};
+ set: #blockStart2 for: #SHTextStylerST80 to: {self dbBedrock};
+ set: #blockEnd2 for: #SHTextStylerST80 to: {self dbBedrock};
+ set: #blockStart3 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #blockEnd3 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #blockStart4 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #blockEnd4 for: #SHTextStylerST80 to: {self dbPurple muchLighter};
+ set: #blockStart5 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #blockEnd5 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #blockStart6 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #blockEnd6 for: #SHTextStylerST80 to: {self dbOrange muchLighter};
+ set: #blockStart7 for: #SHTextStylerST80 to: {Color yellow};
+ set: #blockEnd7 for: #SHTextStylerST80 to: {Color yellow};
- set: #leftParenthesis for: #SHTextStylerST80 to: {self dbBedrock};
- set: #rightParenthesis for: #SHTextStylerST80 to: {self dbBedrock};
- set: #leftParenthesis1 for: #SHTextStylerST80 to: {self dbGreen};
- set: #rightParenthesis1 for: #SHTextStylerST80 to: {self dbGreen};
- set: #leftParenthesis2 for: #SHTextStylerST80 to: {self dbPurple};
- set: #rightParenthesis2 for: #SHTextStylerST80 to: {self dbPurple};
- set: #leftParenthesis3 for: #SHTextStylerST80 to: {self dbRed};
- set: #rightParenthesis3 for: #SHTextStylerST80 to: {self dbRed};
- set: #leftParenthesis4 for: #SHTextStylerST80 to: {self dbGreen};
- set: #rightParenthesis4 for: #SHTextStylerST80 to: {self dbGreen};
- set: #leftParenthesis5 for: #SHTextStylerST80 to: {self dbOrange};
- set: #rightParenthesis5 for: #SHTextStylerST80 to: {self dbOrange};
- set: #leftParenthesis6 for: #SHTextStylerST80 to: {self dbPurple};
- set: #rightParenthesis6 for: #SHTextStylerST80 to: {self dbPurple};
- set: #leftParenthesis7 for: #SHTextStylerST80 to: {self dbBlue};
- set: #rightParenthesis7 for: #SHTextStylerST80 to: {self dbBlue};
- set: #blockStart for: #SHTextStylerST80 to: {self dbBedrock};
- set: #blockEnd for: #SHTextStylerST80 to: {self dbBedrock};
- set: #blockStart1 for: #SHTextStylerST80 to: {self dbGreen};
- set: #blockEnd1 for: #SHTextStylerST80 to: {self dbGreen};
- set: #blockStart2 for: #SHTextStylerST80 to: {self dbPurple};
- set: #blockEnd2 for: #SHTextStylerST80 to: {self dbPurple};
- set: #blockStart3 for: #SHTextStylerST80 to: {self dbRed};
- set: #blockEnd3 for: #SHTextStylerST80 to: {self dbRed};
- set: #blockStart4 for: #SHTextStylerST80 to: {self dbGreen};
- set: #blockEnd4 for: #SHTextStylerST80 to: {self dbGreen};
- set: #blockStart5 for: #SHTextStylerST80 to: {self dbOrange};
- set: #blockEnd5 for: #SHTextStylerST80 to: {self dbOrange};
- set: #blockStart6 for: #SHTextStylerST80 to: {self dbPurple};
- set: #blockEnd6 for: #SHTextStylerST80 to: {self dbPurple};
- set: #blockStart7 for: #SHTextStylerST80 to: {self dbBlue};
- set: #blockEnd7 for: #SHTextStylerST80 to: {self dbBlue};
  set: #arrayStart for: #SHTextStylerST80 to: {self dbBedrock};
  set: #arrayEnd for: #SHTextStylerST80 to: {self dbBedrock};
  set: #arrayStart1 for: #SHTextStylerST80 to: {self dbForeground};
  set: #arrayEnd1 for: #SHTextStylerST80 to: {self dbForeground};
  set: #byteArrayStart for: #SHTextStylerST80 to: {self dbForeground};
  set: #byteArrayEnd for: #SHTextStylerST80 to: {self dbForeground};
  set: #byteArrayStart1 for: #SHTextStylerST80 to: {self dbForeground};
  set: #byteArrayEnd1 for: #SHTextStylerST80 to: {self dbForeground};
  set: #leftBrace for: #SHTextStylerST80 to: {self dbForeground};
  set: #rightBrace for: #SHTextStylerST80 to: {self dbForeground};
  set: #cascadeSeparator for: #SHTextStylerST80 to: {self dbForeground};
  set: #statementSeparator for: #SHTextStylerST80 to: {self dbForeground};
  set: #externalCallType for: #SHTextStylerST80 to: {self dbForeground};
  set: #externalCallTypePointerIndicator for: #SHTextStylerST80 to: {self dbForeground};
  set: #primitiveOrExternalCallStart for: #SHTextStylerST80 to: {self dbForeground};
  set: #primitiveOrExternalCallEnd for: #SHTextStylerST80 to: {self dbForeground};
  set: #methodTempBar for: #SHTextStylerST80 to: {self dbBedrock};
  set: #blockTempBar for: #SHTextStylerST80 to: {self dbBedrock};
  set: #blockArgsBar for: #SHTextStylerST80 to: {self dbBedrock};
  set: #primitive for: #SHTextStylerST80 to: {self dbGreen lighter. bold};
  set: #pragmaKeyword for: #SHTextStylerST80 to: {self dbGreen. bold};
  set: #pragmaUnary for: #SHTextStylerST80 to: {self dbGreen. bold};
  set: #pragmaBinary for: #SHTextStylerST80 to: {self dbGreen. bold};
  set: #externalFunctionCallingConvention for: #SHTextStylerST80 to: {self dbGreen. bold};
  set: #module for: #SHTextStylerST80 to: {self dbGreen. bold};
  set: #blockTempVar for: #SHTextStylerST80 to: {self dbLocal. italic};
  set: #blockPatternTempVar for: #SHTextStylerST80 to: {self dbLocal. italic};
  set: #instVar for: #SHTextStylerST80 to: {self dbYellow. normal };
  set: #workspaceVar for: #SHTextStylerST80 to: {self dbLocal. italic};
  set: #undefinedIdentifier for: #SHTextStylerST80 to: {self dbInvalid};
  set: #incompleteIdentifier for: #SHTextStylerST80 to: {self dbGray. underlined};
  set: #tempVar for: #SHTextStylerST80 to: {self dbLocal. italic};
  set: #patternTempVar for: #SHTextStylerST80 to: {self dbLocal. italic};
  set: #poolConstant for: #SHTextStylerST80 to: {self dbConstant };
  set: #classVar for: #SHTextStylerST80 to: {self dbReference};
  set: #globalVar for: #SHTextStylerST80 to: {self dbClass. normal}.
  "And the text differ"
  aUserInterfaceTheme
  set: #insertTextAttributes for: #TextDiffBuilder to: { TextColor color: self dbRed };
  set: #removeTextAttributes for: #TextDiffBuilder to: { TextEmphasis struckOut. TextColor color: self dbBlue };
  set: #normalTextAttributes for: #TextDiffBuilder to: { TextEmphasis normal }.!

Item was changed:
  ----- Method: CommunityTheme class>>dbRed (in category 'colors') -----
  dbRed
+ ^Color r: 0.75 g: 0.25 b: 0.25!
- ^Color r: 0.6 g: 0.3 b: 0.3!

Item was added:
+ ----- Method: SmalltalkImage>>patchSystem (in category 'command line') -----
+ patchSystem
+ (FileDirectory default fileExists: 'patch.st') ifTrue:
+ [Notification signal: 'Patching system...'.
+ FileStream
+ fileNamed: 'patch.st'
+ do: [ : stream | stream fileIn ] ]!

Item was changed:
  ----- Method: SmalltalkImage>>run: (in category 'command line') -----
  run: aBlock
+ [ [ self patchSystem.
+ (aBlock numArgs = 1 and: [ self arguments size > 1 ])
- [ [ (aBlock numArgs = 1 and: [ self arguments size > 1 ])
  ifTrue: [ "Allow a large, variable number of arguments to be passed as an Array to aBlock."
  aBlock value: self arguments ]
  ifFalse: [ aBlock valueWithEnoughArguments: self arguments ] ]
  on: ProgressInitiationException
  do:
+ [ : pie | "Don't want to log this notification."
- [ : pie | "Don't want to log these notifications."
  pie defaultAction ] ]
  on: Notification , Warning
  do:
  [ : noti | FileStream stdout
  nextPutAll: DateAndTime now asString ;
  space ;
  nextPutAll: noti description ;
  cr.
  noti resume ]
  on: SyntaxErrorNotification
  do:
  [ : err | FileStream stdout
  nextPutAll: err errorCode ;
  cr; flush.
  self isHeadless
  ifTrue: [ self snapshot: false andQuit: true ]
  ifFalse: [ err pass ] ]
+ on: Error
- on: Error, MessageNotUnderstood, Halt
  do:
  [ : err | err printVerboseOn: FileStream stderr.
  FileStream stderr flush.
  self isHeadless
  ifTrue: [ self snapshot: false andQuit: true ]
  ifFalse: [ err pass ] ]!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

Tobias Pape
Hi

Where's the problem with calling the image with 'patch.st' in the first place?
It looks a little bit out of place for me here.
Also, Why Kill MNU and Halt from the list?
I'm a bit confused about this part of the pathc…

Best regards
        -tobias
On 31.08.2016, at 23:33, [hidden email] wrote:

> Item was added:
> + ----- Method: SmalltalkImage>>patchSystem (in category 'command line') -----
> + patchSystem
> + (FileDirectory default fileExists: 'patch.st') ifTrue:
> + [Notification signal: 'Patching system...'.
> + FileStream
> + fileNamed: 'patch.st'
> + do: [ : stream | stream fileIn ] ]!
>
> Item was changed:
>  ----- Method: SmalltalkImage>>run: (in category 'command line') -----
>  run: aBlock
> + [ [ self patchSystem.
> + (aBlock numArgs = 1 and: [ self arguments size > 1 ])
> - [ [ (aBlock numArgs = 1 and: [ self arguments size > 1 ])
>   ifTrue: [ "Allow a large, variable number of arguments to be passed as an Array to aBlock."
>   aBlock value: self arguments ]
>   ifFalse: [ aBlock valueWithEnoughArguments: self arguments ] ]
>   on: ProgressInitiationException
>   do:
> + [ : pie | "Don't want to log this notification."
> - [ : pie | "Don't want to log these notifications."
>   pie defaultAction ] ]
>   on: Notification , Warning
>   do:
>   [ : noti | FileStream stdout
>   nextPutAll: DateAndTime now asString ;
>   space ;
>   nextPutAll: noti description ;
>   cr.
>   noti resume ]
>   on: SyntaxErrorNotification
>   do:
>   [ : err | FileStream stdout
>   nextPutAll: err errorCode ;
>   cr; flush.
>   self isHeadless
>   ifTrue: [ self snapshot: false andQuit: true ]
>   ifFalse: [ err pass ] ]
> + on: Error
> - on: Error, MessageNotUnderstood, Halt
>   do:
>   [ : err | err printVerboseOn: FileStream stderr.
>   FileStream stderr flush.
>   self isHeadless
>   ifTrue: [ self snapshot: false andQuit: true ]
>   ifFalse: [ err pass ] ]!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

Chris Muller-3
Hi Tobias,

> Where's the problem with calling the image with 'patch.st' in the first place?

Per our other conversation in box-admins, I want to control the RFB
configuration from the Linux side.  Once the server image builds
itself and saved once, it is never saved again.

If we can't save the image, how to do a quick-patch?  In today's
SqueakSource, we RFB into the image, make the method changes, save the
image.  (Shudder).  After a week nobody remembers what the heck we're
running in production, and its not easy to see even when one gets
RFB'd into the image.

In this new SqueakSource server setup, the built image is never
resaved, so its validity could actually be easily confirmed with a
hashsum.

The image is never resaved, but could still suffer a daemontools
restart, in which case the patch must be reapplied automatically.
This is done by the administrator exporting the method changes as
patch.st, instead of saving the image.

> It looks a little bit out of place for me here.

It should be private.  I'll do that.

> Also, Why Kill MNU and Halt from the list?

MNU is covered by Error.  Halt because its sometimes necessary to use
it to diagnose and fix a production problem.

Best,
  Chris

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

Tobias Pape

On 01.09.2016, at 05:37, Chris Muller <[hidden email]> wrote:

> Hi Tobias,
>
>> Where's the problem with calling the image with 'patch.st' in the first place?
>
> Per our other conversation in box-admins, I want to control the RFB
> configuration from the Linux side.  Once the server image builds
> itself and saved once, it is never saved again.

I still do not get this.

>
> If we can't save the image, how to do a quick-patch?  In today's
> SqueakSource, we RFB into the image, make the method changes, save the
> image.  (Shudder).  After a week nobody remembers what the heck we're
> running in production, and its not easy to see even when one gets
> RFB'd into the image.
>
> In this new SqueakSource server setup, the built image is never
> resaved, so its validity could actually be easily confirmed with a
> hashsum.
>
> The image is never resaved, but could still suffer a daemontools
> restart, in which case the patch must be reapplied automatically.
> This is done by the administrator exporting the method changes as
> patch.st, instead of saving the image.

Well, then we should write the startup script in a manner that it always is started with
a script, which _then_ gets amended outside[1].
IMHO, The hook is providing a script in the first place, not relying a magic constant file name.

>
>> It looks a little bit out of place for me here.
>
> It should be private.  I'll do that.
>
>> Also, Why Kill MNU and Halt from the list?
>
> MNU is covered by Error.  Halt because its sometimes necessary to use
> it to diagnose and fix a production problem.

Yeah, but to my knowledge, run: has up to now be intended for non-interacive use.
_Not_ catching Halt defeats this purpose.

Best regards
        -Tobias

>
> Best,
>  Chris
>

[1]: I am still not comfortable with dropping on the image concept just like that :(


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

marcel.taeumel
In reply to this post by commits-2
Hi Chris,

here are some thoughts on this commit:

- Please refrain from committing unrelated changes that are easily to separate. If you have troubles uploading the 1000K MCZ for the System package twice, consider upgrading your ISP. :-) *scnr*
- That "patch.st" hack seems like an indication that there are several situations where you cannot cope with the .image characteristics of the Smalltalk system. Please think about a better solution because the .image file should usually not be read-only. It impedes Smalltalk-style maintenance of such an image. While there have been good reasons in the past for treating the .image in the Etoys bundles as read-only and relying on project import/explort, I do not agree that we should make such a specific case common practice. This "patch.st" feels like a first step towards yet another CI/build system, which we already have with smalltalkCI and TravisCI.
- This commit message would benefit from a little more explanation. There is plenty "what you did" but too little "why you did it". For example, the words "... to be re-patched in the event of a restart" really need an explanation of why you cannot just supply a start-up script to do that patching like "./vm some.image patch.st". Please invest more time in more elaborate commit messages.

:)

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

Chris Muller-3
Hi Marcel,

> - Please refrain from committing unrelated changes that are easily to
> separate. If you have troubles uploading the 1000K MCZ for the System

I do keep significant changes, which are unrelated to each other, to
separate commits, however, I prefer to piggy-back my meaningless
changes like color tweaks or comment typo's.  The reason is, when I
arrow through the MC history of a package, I only want to see
*meaningful* changes, not dozens of meaningless tiny little fixes and
roll-backs that don't need to be consumed later, therefore, don't need
to be in their own commit.

This isn't even about the heavyweightness and unscalability of MC,
which is an issue in itself, but purely about having a dense and tidy
MC history domain.

MC is able to handle all the diffing, merging, backporting, etc. in
spite of this.  You didn't explain why you prefer to have thousands of
micro versions...

> - That "patch.st" hack seems like an indication that there are several
> situations where you cannot cope with the .image characteristics of the
> Smalltalk system. Please think about a better solution because the .image
> file should usually not be read-only. It impedes Smalltalk-style maintenance
> of such an image. While there have been good reasons in the past for
> treating the .image in the Etoys bundles as read-only and relying on project
> import/explort, I do not agree that we should make such a specific case
> common practice. This "patch.st" feels like a first step towards yet another
> CI/build system, which we already have with smalltalkCI and TravisCI.

I don't know how to respond to these comments other than -- yesterday
when you spoke of avoiding the image from growing into a "beast" -- I
totally agree and that's why, IMO, all Squeak-based servers should use
this design, because that's what it does.

> - This commit message would benefit from a little more explanation. There is
> plenty "what you did" but too little "why you did it". For example, the
> words "... to be re-patched in the event of a restart" really need an
> explanation of why you cannot just supply a start-up script to do that
> patching like "./vm some.image patch.st". Please invest more time in more
> elaborate commit messages.

I think the issue is that I should write some documentation about
making a Squeak server using this design.  That's where the answer to
such question would be found..  MC commits only should have enough
information to understand the context of the *change*, i.e., "what's
different", not any broader explanations of how the system works.

BTW, the answer(s) to that question are:  1) because nearly every
single script requires its own ".st" counterpart already, 2)
daemontools runs the #run script, not #patch, and 3) I require to be
able to ascertain exactly what I'm running in production from the
command-line, without having RFB' into the image and start diffing the
dirty MC packages in the production server...

Best,
  Chris

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cmm.913.mcz

Nicolas Cellier


2016-09-01 21:42 GMT+02:00 Chris Muller <[hidden email]>:
Hi Marcel,

> - Please refrain from committing unrelated changes that are easily to
> separate. If you have troubles uploading the 1000K MCZ for the System

I do keep significant changes, which are unrelated to each other, to
separate commits, however, I prefer to piggy-back my meaningless
changes like color tweaks or comment typo's.  The reason is, when I
arrow through the MC history of a package, I only want to see
*meaningful* changes, not dozens of meaningless tiny little fixes and
roll-backs that don't need to be consumed later, therefore, don't need
to be in their own commit.

This isn't even about the heavyweightness and unscalability of MC,
which is an issue in itself, but purely about having a dense and tidy
MC history domain.

MC is able to handle all the diffing, merging, backporting, etc. in
spite of this.  You didn't explain why you prefer to have thousands of
micro versions...

Hi Chris,
the main reasons for using atomic commits:
The ability to review things - large blob of unrelated things are difficult to review
The ability to know which changes go together if we want to backport/revert a patch/feature
 and eventually to cherry pick changes from one branch to another branch
 (more a git feature than mc, but could be done in mc too).

Separation of concerns: putting different features into different commits is just like housekeeping,
like classifying classes in right categories and methods in right protocols...
This is not strictly necessary but help following the stream of changes.

At extreme level where branching is easy like git, one well accepted policy is to put each feature into a different branch.
In mc, where each parallel commit is in its own branch (no matter the naming conventions),
this is somehow what we do (or at least should do) when committing in inbox.
This does ease the work of integrating those changes.

Why should the policy be different in trunk?

 
> - That "patch.st" hack seems like an indication that there are several
> situations where you cannot cope with the .image characteristics of the
> Smalltalk system. Please think about a better solution because the .image
> file should usually not be read-only. It impedes Smalltalk-style maintenance
> of such an image. While there have been good reasons in the past for
> treating the .image in the Etoys bundles as read-only and relying on project
> import/explort, I do not agree that we should make such a specific case
> common practice. This "patch.st" feels like a first step towards yet another
> CI/build system, which we already have with smalltalkCI and TravisCI.

I don't know how to respond to these comments other than -- yesterday
when you spoke of avoiding the image from growing into a "beast" -- I
totally agree and that's why, IMO, all Squeak-based servers should use
this design, because that's what it does.

> - This commit message would benefit from a little more explanation. There is
> plenty "what you did" but too little "why you did it". For example, the
> words "... to be re-patched in the event of a restart" really need an
> explanation of why you cannot just supply a start-up script to do that
> patching like "./vm some.image patch.st". Please invest more time in more
> elaborate commit messages.

I think the issue is that I should write some documentation about
making a Squeak server using this design.  That's where the answer to
such question would be found..  MC commits only should have enough
information to understand the context of the *change*, i.e., "what's
different", not any broader explanations of how the system works.

BTW, the answer(s) to that question are:  1) because nearly every
single script requires its own ".st" counterpart already, 2)
daemontools runs the #run script, not #patch, and 3) I require to be
able to ascertain exactly what I'm running in production from the
command-line, without having RFB' into the image and start diffing the
dirty MC packages in the production server...

Best,
  Chris