[Review]: Issue 15507: SliceMaker Overhaul

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

[Review]: Issue 15507: SliceMaker Overhaul

Sean P. DeNigris
Administrator
Issue 15507: SliceMaker Overhaul
https://pharo.fogbugz.com/default.asp?15507

It all started simply enough: I wanted to extract the issue title downloading - which is generally useful - from slice maker. One full day later, I ended up with a pretty extensive rewrite. Now, I feel like it would've been better to start from scratch, but IMHO it's better, and good enough to move on to more pressing things.

Fix in inbox: SLICE-Issue-15507-SliceMaker-Overhaul-SeanDeNigris.1

###[FEATURE]: PharoIssue class. Here is it's class comment:
I represent a Pharo development issue.

Responsibilities:
- create text templates (e.g. for the mailing list or issue tracker comments) for common events like issue creation and slice submission
- communicate with the issue tracker

Collaborators:
- ZnClient - my link to the issue tracker
- MCSliceInfo - I can get you one if you need it

Public API and Key Messages:
Most of my behavior is illustrated in my tests. The noticable exception are my text template creation messages, the testing of which seemed to hurt as much in duplication as it added in QA.

###[Cleanup]: Komitter - Remove Duplication Caused By MCSliceInfo
- For some reason, Komitter seems to deal with MCPackages, while MCSliceInfo deals with MCWorkingCopy instances. This causes a tiny difference when making a slice (one sends #package to the wc, the other already has the package). The previous solution was to copy/paste a long method, changing one statement. This one trades that method in for "MCPackage>>#package ^ self" with a careful explanation in the comment of why we are doing this ugly thing!

###[ENH]: MC Slice Making - This is the most significant change
- Extract issue-related behavior to PharoIssue
- Extract slice-related behavior to MCSliceInfo. This class is still pretty confusing. It's a cross between  a model of an actual slice, and an Application Model for MCSliceMaker. But since slices are supposed to go away soon...
- Many small cleanups - fix misspellings, improve comments, simplify code
- Remove the "Grab" button. The issue title is automatically downloaded when the issue number field loses focus
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: [Review]: Issue 15507: SliceMaker Overhaul

Sean P. DeNigris
Administrator
Sean P. DeNigris wrote
Issue 15507: SliceMaker Overhaul
https://pharo.fogbugz.com/default.asp?15507
Passed validation (Issue Validation Succeeded: https://ci.inria.fr/pharo/job/Pharo-5.0-Issue-Validator/1579//artifact/validationReport.html)!

Our CI is really cool :) I accidentally noticed and fixed a pre-existing changes dependency leak in the Critic Browser while I was fixing the other rules I broke with my fix.
Cheers,
Sean