EYES ON CHECK IN, EYES ON THE CODE

1. EYES ON CHECK IN, EYES ON THE CODE POLICING OR LIBERATING A WORKPLACE THROUGH CODE INSPECTION? ILPC, Dublin, 19 March 2008 Student: Allen Higgins Supervisor: Joe Nandhakumar

2. Research Motivation: Understanding Intersubjectivity in Design Work How do programmers and others: • Establish meaning surrounding the software during periods where the source code is open-ended and undergoing development. • Develop and employ infrastructures and techniques used to carry out work (in collaborative and collective modes). • Perform the work of design through activity systems spanning many actors. Demands a focus on the mundane yet often ignored practices engaged by participants in the development and maintenance of software. Inform the organisation and practice of: collective collaborative (creative?) …software design work

3. Labour Process, an orientation to the field & data gathering • Braverman (after Taylor) develops Marx’s labour process analysis by attending to the conditions of the individual worker and processes of work itself • The progressive alientation of the process of production anticipates that control over labour process passes from the hands of worker into management (and by definition capital), and for them to exploit their power to increase labour output without check • Anticipate data for labour process method (Marx 1974 (1867):173), to consist of descriptions of: • activity – the organisation of labour • the instruments of labour • the changes intentionally effected • ascertain interests of labour and capital, ownership of tools, of product… • locate where control resides, oppositional histories

4. Research site Crucible: An Irish software publishing house, 25 employees • Longitudinal case study (commenced involvement 2004), participant observation. Researcher present in role of internal quality auditor Worker profile 24 employees (varied +/- 5), 4f 20m 20-30yrs (8), 30-40yrs (11), 40-50yrs (5) Management team (1f 20+, 1m 30+, 3m 40+) 3 Work teams organised on functional lines but some overlap Engineering (& Support) – 5 Test (& Support) – 2 Support (& Consulting) – 5 (3 distant workers) Sales (& Consulting) – 5 (n.b. all separate distant workers) Operations – 7 (n.b. not organised as a work team) • This field data set relates to over 50 hours of interviews and recorded observations

5. Organisation Chart

6. Crucible • Produces software for process industry, factory automation – data and historical analysis software product, highly configurable • In-house development processes and product lifecycle • Emphasis on new product development • Undercurrent of maintenance work • Quality Management System ISO 9000:2001, automated manufacturing practice standard compliance, focus on • Plan-Do-Check-Act • Evidence based assessment, regular audits • SOPs and documentation

7. The QMS and Code Review 2001-2002

8. The QMS and Code Review 2003-2004

9. The First Code Review Process • After Fagan’s vision for code inspection (Fagan, 1976) • Moderator • Designer • Coder/implementer • Tester • Preparation, Inspection, Rework, Follow-up Except that Fagan’s vision was also to ‘instrument’ the activity (ranking, classification, numeric analysis, error density etc) Code walk throughs are similar though not so highly structured. Fagan, M.E. "Design and Code Inspections to Reduce Errors in Program Developmen," IBM Systems Journal (15:3) 1976,

10. Customers Expect so Customers Get: Code Reviews

11. General Comments More comments needed A small description of each type of filter would be useful in the file, maybe reference a few resources on the web in the comments as well? Standard in the rest of PRODUCT is to have _E rather than E_ FILTER_FAIL is unused anywhere; get rid of it to save the client having to test for this. Line specific comments CoFileTimeNow() being called but the value isn’t used anywhere? Set result to some default value like fail, reduces bugs. Use of magic number 0x00000000deadbeef, use a const. The comment “or there is no time difference” does not match the logic – the logic says diff >= 0 not diff > 0. timeDiff * 1.0 causes a cast to double, someone could remove this thinking it’s not necessary; put in an explicit comment that this is intentional. If a const char* is used as a template, there is no error handling. There is no need to use a VALUETYPE as the value counter, an int would suffice Conventional Code Review Document

12. Negotiating Something They’ll Use

13. Suggesting ‘eyes’

14. Eyes on check-in, Subversion (SVN) comments Merged changes from http://server/svn/PRODUCT/branches/5.2.x/src rev. 24151 - 24153 - Add logic in GetItemHandleImpl to prevent clutter in the Event log when XXX Server is queried by the Web Client. - XXX Server does not recover if commit trans fails - Message suppression included in XXX Server - Add OPCXXX AsyncRead and Playback Playback interfaces - Server does not return an error in GetItemAttributes when not licensed. - Add exception handling macros for IOPCXXX_Browser, IOPCCommon, IOPCXXX_SyncRead, IOPCXXX_Server interfaces - Convert remaining CustomLog calls to new formatting. - Time average aggregate not correctly calculated where interval ends with bad quality value - Change REG_DATALOGGER_SUPPRESSE to REG_SUPPRESSED Issue: 6519 Issue: 6520 Issue: 6521 Issue: 6571 eyes: Brown Issues: Issue #6519: http://server/mantis/view.php?id=6519 Issue #6520: http://server/mantis/view.php?id=6520 Issue #6521: http://server/mantis/view.php?id=6521 Issue #6571: http://server/mantis/view.php?id=6571

15. Brief ‘eyes’ vignettes Pink: Exactly (.) annd, then, because of the very generic way in which we do it, we can actually merge data sources (at that point) ((typing, clicking, mousing on screen)). So if we take the... Orange: ...need another... Pink : item displays, take this guy, give it a (.) dedicated axis. (#3) We can see that it doesn't work ((ironic flourish announces))! ((they don't see what we expected to see on screen, fixed axis wrong, data presentation wrong)) Orange : hhmm Pink : That's very interesting isn't it. ((typing, clicking, mousing on screen)). Did I actually do that? Oh wait, (.) ha! Sorry, that's the x-axis. Orange : ah hum Brown: And we’re getting the status, the status, //the status Green: //yeah Brown: the connection status, and this variable… Green: One is actually connected to the icon, and it //calls Brown: Yeah, and these values? Green: Yup, ahh, that’s bad as ((quiet typing))

16. Brief ‘eyes’ vignettes Pink: (softly) Guys, ehm, see XYZSystemDialog, when was that checked in? Green: Oh, let me… Pink: Is that ah, when was that checked in? Green: It was checked in, ah, Friday? Pink: Friday yeah, cause (querulous) I’m getting an unresolved external symbol, where should I beee.. Green: ..is it the file or is it the ID? Pink: (louder) its an unresolved symbol so that it’s the destructor and the construct. Green: (inaudible) You’re missing it in the project file, ((keyboard noise)) or the config file needs to be updated or your, you’ll need a fetch Pink: Right Green: Its just not included in your project file Pink: I’ve done, I’ve done an update. Maybe a full refresh. I know its building it’s just something is missing on my machine Pink: (softly) There, usually… (louder) Well that’s sorted it, I just restarted, reopened my folder (querulously) usually it says something like ‘configuration project needs updating\\ Green: Yeah, right. Yeh.

17. Reflections Brown: its not programming, its design Pink: no no, its programming, you’re thinking and programming at the same time White: you see ‘eyes’ has picked up more design issues than code reviews ever would, its early enough. It’s also a great way to understand other areas of the code that someone’s working on. Pink: Talking about the idea of the software is more than anything else. We really don’t write a lot. What matters is talking about it. The most important thing isn’t the source; it’s us, the people who write it. We could start again tomorrow and we’d be up and running. There’s no way you’d do that starting fresh with new people on the same codebase.

18. Discussion • Code review processes imply the encoding of technocratic modes of thinking, so fitting nicely into Taylor’s and Braverman’s understanding of management posessing knowledge and control of production processes (Alvesson et al. 1996) • But labour process analyses may miss the critical role played by interpretive processes of actors, “whether such rules are deployed or not depends on who interprets them and how they are interpreted.” (Grint, 1998) • The technology of the code review SOP may not in fact act as a control mechanism, even if it was intended as such. • And control may arise in other ways for other reasons; the concertive control of teams by their own members may come into play, but without reinventing bureaucratic structures observed in other autonomous modes of production (Barker 1998) Alvesson, M., and Willmott, H. Making Sense of Management: A Critical Introduction Sage Publications, London, 1996 Grint, K. The Sociology of Work, (2nd Edition ed.) Polity, 1998. Barker, J.R. "Tightening the Iron Cage: Concertive Control in Self-Managing Teams," in: Qualitative Studies in Organizations, Theory and Behaviour, J.V. Maanen (ed.), 1998

19. Conclusions • Analysis of the data from our field studies points to the importance of attending to the dynamics and flow of work practice in actual workplaces. • Labour process analysis presents both a methodological approach and theoretical orientation to understanding work. • The labour process is an obvious starting point for the analysis of workplace organisation because it focuses attention on the practices and processes of work itself. • Labour process theory is also designed to explicate conventional managerialist framing of the control of production, to draw attention to asymmetries of power and oppositional tendencies. • In this case peer performed code reviews capture a moment of managerial intention; the surveillance of production, assessment of quality, and conformance to standards. These moments present opportunities for the activities of management, for example: assessment, recognition, reward, correction, sanction

21. Additional slides

22. The Problem of Organising Programming in Teams The problem of scaling up from individualist work to organising collaborative collective work; from cognitive work to co-production, from individuals to teams (of individuals) Brooks (1975) Surgical Team… employed ‘conferences and courts’ as forums for discussion; emphasising the sharing of knowledge, creativity, reaching consensus and decision making for these groups. “writing code isn’t the problem, understanding the problem is the problem” Curtis et al. (1988) “In general the discussions… were informal. Issues… were not discussed hierarchically, but via free-flowing, unstructured strings of quasi-related episodes. A discussion of one issue seemed to trigger the discussion of new issues.” (Walz et al. 1993, p. 72). Curtis, B., Krasner, H., and Iscoe, N. "A Field Study of the Software Design Process for Large Systems," Communications of the ACM (31:11) 1988. Walz, D.B., Elam, J.J., and Curtis, B. "Inside a Software Design Team: Knowledge Acquisition, Sharing, and Integration," Communications of the ACM

23. Approaching the Field • The labour process “ends in the creation of something, which when the process began, already existed in the worker’s imagination, already existed in an ideal form.” (Marx 1974 (1867):170) • Marx’s labour process implies the division of labour from the outset as the outputs or tools produced in one process are used or consumed through another • Trained, skilled knowledgeable labour is reflected simply in greater use values (ibid:192) • Taylor took seriously the role of the individual worker in producing labour, by focusing on the content of work he surfaced assumptions surrounding its manageability (Taylor 1917 (1911)) • But made it manageable by taking responsibility from the worker, replacing the judgement of the individual worker with a planning room staffed with expert teachers and “time-study men” Marx, K. Capital, 1974 (1867) Taylor, F.W. The Principles of Scientific Management 1917 (1911)

24. The process of ‘eyes’ • ‘Eyes’ is talked about by the programmers as their own code review procedure. • The ‘house rule’ is that no code can be added to or changed in the central source base unless it has ‘eyes’ attached (as comment). • It has become an integral part of day to day working practice of developers working in the firm. • The procedure appears to be a simple ‘quick and dirty’ peer review of each other’s work rather than the more conventional ‘heavy-weight’ code review prescribed in the literature. • As an ‘eyes’ session unfolds, code changes and additions are talked through, discussed, challenged and modified, changes are made on-the-fly or might be marked for later attention

25. Conclusions • Braverman’s vision was not entirerly bleak • “An automatic system of machinery opens up the possibility of the true control over a highly productive factory by a relatively small corps of workers, providing these workers attain the level of mastery over the machinery offered by engineering knowledge, and providing they then share out among themselves the routines of the operation, from the most technically advanced to the most routine.” • He hoped they would become “freely associated producers who control their own labor and their own destinies,” (Braverman, 1998:159-161) Braverman, H. Labor and monopoly capital : the degradation of work in the twentieth century, (25th anniversary edition ed.) Monthly Review Press, New York, 1998