The Trunk: Graphics-pre.405.mcz

Hi Levente,

The issue at large here (and what the swiki updates are about) is
management of quality ancestry and code repository.  I'm asking you to
care about the "content" of Squeak that lies beyond the current
running code in the image memory.  We have had on-going and very
tiring discussions through the years, particularly when spirit and
letter of the "New Community Development Model" was violated.  These
three:

* Exercise caution. This is a running system and breaking it
needlessly is generally frowned upon.
* Restrain yourself. Getting developer access doesn’t mean you are
free to put in every pet extension you always wanted to have without
discussion.
* If in doubt, ask. T....

very much resemble what I wrote on the swiki.  And they appear before:

* You break it, you fix it.

Reading between the lines, "You break it, you fix it" is the
*last-resort* when the first three failed.

If someone submits a whimsical, same-day change to low-level,
high-impact methods with ZERO testing, breaks the image, then is
immediately "undone" with another commit, then it is going to get
admin'd.  It happens.  In this instance, please do not feign like
purging litter caused harm.   It was the original whimsical same-day
low-level method commit that caused the harm, including having these
endless discussions about it instead of just reverting one package and
saying "thanks, sorry for the goof up, next time I'll use the Inbox."
  :(

On Wed, Oct 17, 2018 at 2:52 PM Levente Uzonyi <[hidden email]> wrote:
>
> Hi Chris,
>
> I was wondering where that list of rules came from, because they don't
> even resemble the actual Trunk developement rules[1].
> So, I checked the entry and found that the list was added to the wiki this
> July[2].
> I don't remember any discussion about changing the Trunk developement
> rules. Was there such discussion? If so, where can I find it?
>
> Levente
>
> [1] https://squeakboard.wordpress.com/2009/07/02/a-new-community-development-model/
> [2] http://wiki.squeak.org/squeak/3279.diff?id=60
>
> On Tue, 16 Oct 2018, Chris Muller wrote:
>
> > I've gone and moved these two inadvertent commits to the Treated Inbox.
> >
> > If you'd already updated your trunk image in the last few hours,
> > revert your image to Graphics-pre.403.
> >
> > Since we're starting fresh on a new development cycle, please take
> > this opportunity to review Guidelines for Core Developers for Trunk
> > submissions:
> >
> >    http://wiki.squeak.org/squeak/3279
> >
> > Regards,
> >  Chris
> >
> > ___________________
> > 1.  Commit it to the Inbox first.
> > 2.  Regard the code repository with equal respect to the image, for
> > example, by avoiding unnecessary litter and bloat.
> > 3.  Never only change formatting, and even avoid changing prior
> > developers formatting when making only minor changes to a method.
> > 4.  Give your peers a chance to review, commit to the Inbox first.
> > 5.  Never commit same-day code. Think about it and live with it for at
> > least a few days first.
> > 6.  Share it with others for extra review eyes. The Inbox is a great
> > way to do this.
> > 7.  Only commit meaningful changes like a fix or improvement, not
> > merely a spelling or comment fix. Gather those up to include the next
> > time there is a meaningful change to that package.
> >
> >
> > On Tue, Oct 16, 2018 at 11:08 AM <[hidden email]> wrote:
> >>
> >> Patrick Rein uploaded a new version of Graphics to project The Trunk:
> >> http://source.squeak.org/trunk/Graphics-pre.405.mcz
> >>
> >> ==================== Summary ====================
> >>
> >> Name: Graphics-pre.405
> >> Author: pre
> >> Time: 16 October 2018, 5:07:10.382369 pm
> >> UUID: 75b41799-4dd2-7a44-b98f-89612c4d7af9
> >> Ancestors: Graphics-pre.404
> >>
> >> Restores the 32bit black-transparent conversion. The corresponding test has been adapted and set to be an expected failure as the behavior should be tackled for 32bit at one point. The inconsistency results in major issues when working with forms resulting from loading pictures from external resources. An alternative approach would require changing all methods creating forms from external sources, although that would destroy color information.
> >>
> >> =============== Diff against Graphics-pre.404 ===============
> >>
> >> Item was changed:
> >>   ----- Method: Color class>>colorFromPixelValue:depth: (in category 'instance creation') -----
> >>   colorFromPixelValue: p depth: d
> >>         "Convert a pixel value for the given display depth into a color."
> >>         "Details: For depths of 8 or less, the pixel value is simply looked up in a table. For greater depths, the color components are extracted and converted into a color."
> >>
> >>         | r g b alpha |
> >>         d = 8 ifTrue: [^ IndexedColors at: (p bitAnd: 16rFF) + 1].
> >>         d = 4 ifTrue: [^ IndexedColors at: (p bitAnd: 16r0F) + 1].
> >>         d = 2 ifTrue: [^ IndexedColors at: (p bitAnd: 16r03) + 1].
> >>         d = 1 ifTrue: [^ IndexedColors at: (p bitAnd: 16r01) + 1].
> >>
> >>         (d = 16) | (d = 15) ifTrue: [
> >>                 "five bits per component"
> >>                 r := (p bitShift: -10) bitAnd: 16r1F.
> >>                 g := (p bitShift: -5) bitAnd: 16r1F.
> >>                 b := p bitAnd: 16r1F.
> >>                 (r = 0 and: [g = 0]) ifTrue: [
> >>                         b = 0 ifTrue: [^Color transparent].
> >>                         b = 1 ifTrue: [^Color black]].
> >>                 ^ Color r: r g: g b: b range: 31].
> >>
> >>         d = 32 ifTrue: [
> >>                 "eight bits per component; 8 bits of alpha"
> >>                 r := (p bitShift: -16) bitAnd: 16rFF.
> >>                 g := (p bitShift: -8) bitAnd: 16rFF.
> >>                 b := p bitAnd: 16rFF.
> >>                 alpha := p bitShift: -24.
> >>                 alpha = 0 ifTrue: [^Color transparent].
> >> +               (r = 0 and: [g = 0 and: [b = 0]])  ifTrue: [^Color transparent].
> >>                 alpha < 255
> >>                         ifTrue: [^ (Color r: r g: g b: b range: 255) alpha: (alpha asFloat / 255.0)]
> >>                         ifFalse: [^ (Color r: r g: g b: b range: 255)]].
> >>
> >>         d = 12 ifTrue: [
> >>                 "four bits per component"
> >>                 r := (p bitShift: -8) bitAnd: 16rF.
> >>                 g := (p bitShift: -4) bitAnd: 16rF.
> >>                 b := p bitAnd: 16rF.
> >>                 ^ Color r: r g: g b: b range: 15].
> >>
> >>         d = 9 ifTrue: [
> >>                 "three bits per component"
> >>                 r := (p bitShift: -6) bitAnd: 16r7.
> >>                 g := (p bitShift: -3) bitAnd: 16r7.
> >>                 b := p bitAnd: 16r7.
> >>                 ^ Color r: r g: g b: b range: 7].
> >>
> >>         self error: 'unknown pixel depth: ', d printString
> >>   !
> >>
> >>
>

> the issue at stake here is the sanctity of the version history.

"Sanctity."   If only you REALLY felt that way...    :(

> It's untouchable. Think of it as immutable, append-only.

Versions buried more than one or two deep are untouchable.  The top
version(s) are not.  Reverting to a prior version IS a use-case.
Deleting or moving a version IS a use-case.  There are UI commands to
do them.

> Unless some commit actually breaks the update stream or does some other kind of major damage to users systems, it is always preferable to revert that commit by a subsequent commit.

You have a penchant for stating things as "facts" without offering any
rationale whatsoever.  I could do something similar by only
saying,"it's rarely preferable to revert a commit by a subsequent
commit," with no basis.

> Patrick did that, and nobody was adversely affected.

No!  We've been through this but, since you're Bert...   you're
conflating what you think is a "miniscule adverse effect" with "no
adverse effect".   In truth the  __breadth__  of the impact is not
miniscule:

  - *Every client of every user* is adversely affected.
  - *Every dimension of the hardware* is affected (memory, storage, network).
  - And software -- the usability of the code repository and ancestral model.
  - *Every future user* and their hard drives, memorys, and networks
will also be adversely affected.
  - These adverse effects are _permanent_,
            even though they were conjured committed in only minutes
from a whimsy.

Growing the universe by 0.1% is a big deal that advsersely affects
everybody whether they realize it or not.

It was only by admin'ing, that nobody will be adversely affected.

> Our rules about caution, restraint, doubt etc are to prevent these critical problems from occurring, not to keep the version history "pretty".

Stop mischaracterizing the reason for maintaining a quality ancestry
and code repository.

> Also, I agree with Levente that your "inbox first" rule is not actually what we agreed upon. When we promote someone to core developer we trust them enough to judge whether to put stuff into trunk directly or not.

Of course.  That's why it says "Guidelines" not "Rules".  This was
written to instill a conservative approach to committing.  It really
is a good approach for something you consider immutable.

As I said before, "hopefully it won't happen very often, but if it
does..."  Rest assured, I don't enjoy being this "bad guy"...   :(

On Wed, Oct 17, 2018 at 3:00 PM Chris Muller <[hidden email]> wrote:

> the issue at stake here is the sanctity of the version history.

"Sanctity."   If only you REALLY felt that way...    :(

🤔

 

> It's untouchable. Think of it as immutable, append-only.

Versions buried more than one or two deep are untouchable.  The top
version(s) are not.  Reverting to a prior version IS a use-case.
Deleting or moving a version IS a use-case.  There are UI commands to
do them.

Nope. Not in Trunk. Only trunk admins can move versions. Because it is a last-resort option when all else fails.

> Unless some commit actually breaks the update stream or does some other kind of major damage to users systems, it is always preferable to revert that commit by a subsequent commit.

You have a penchant for stating things as "facts" without offering any
rationale whatsoever.  I could do something similar by only
saying,"it's rarely preferable to revert a commit by a subsequent
commit," with no basis.

It's a commonly accepted practices in our field:

"since so much work is local within your clone, you have a great deal of freedom to rewrite your history locally. However, once you push your work, it is a different story entirely, and you should consider pushed work as final unless you have good reason to change it. "

> Patrick did that, and nobody was adversely affected.

No!  We've been through this but, since you're Bert...   you're
conflating what you think is a "miniscule adverse effect" with "no
adverse effect".   In truth the  __breadth__  of the impact is not
miniscule:

  - *Every client of every user* is adversely affected.
  - *Every dimension of the hardware* is affected (memory, storage, network).
  - And software -- the usability of the code repository and ancestral model.
  - *Every future user* and their hard drives, memorys, and networks
will also be adversely affected.
  - These adverse effects are _permanent_,
            even though they were conjured committed in only minutes
from a whimsy.

Growing the universe by 0.1% is a big deal that advsersely affects
everybody whether they realize it or not.

It was only by admin'ing, that nobody will be adversely affected.

Sorry, I did not understand that you are worried about the physical history size.

> Our rules about caution, restraint, doubt etc are to prevent these critical problems from occurring, not to keep the version history "pretty".

Stop mischaracterizing the reason for maintaining a quality ancestry
and code repository.

I did not get that, sorry again. I also do not agree with it. We should fix our tools, not change history.

> Also, I agree with Levente that your "inbox first" rule is not actually what we agreed upon. When we promote someone to core developer we trust them enough to judge whether to put stuff into trunk directly or not.

Of course.  That's why it says "Guidelines" not "Rules".  This was
written to instill a conservative approach to committing.  It really
is a good approach for something you consider immutable.

As I said before, "hopefully it won't happen very often, but if it
does..."  Rest assured, I don't enjoy being this "bad guy"...   :(

In a community as small as ours I think it's more valuable to be proactive than conservative. That was the original vision of the trunk development process - removing the gatekeepers, empowering individual developers. I still think that's a fundamentally good idea.

- Bert -