Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1184)

Issue 284980043: stencil.scm: make args optional in stencil-whiteout (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 2 months ago by pwm
Modified:
8 years, 2 months ago
Reviewers:
thomasmorley651
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

stencil.scm: make args optional in stencil-whiteout

Patch Set 1 #

Total comments: 3

Patch Set 2 : doc string edit and fewer default values #

Total comments: 3

Patch Set 3 : simplify docstring a bit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -6 lines) Patch
M scm/stencil.scm View 1 2 1 chunk +13 lines, -6 lines 0 comments Download

Messages

Total messages: 9
pwm
Please review, thanks. -Paul
8 years, 2 months ago (2016-01-03 21:29:50 UTC) #1
thomasmorley651
Thanks. Some thoughts: https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm#newcode804 scm/stencil.scm:804: (define*-public (stencil-whiteout stil #:optional (style 'box) ...
8 years, 2 months ago (2016-01-03 23:14:22 UTC) #2
thomasmorley651
https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm#newcode804 scm/stencil.scm:804: (define*-public (stencil-whiteout stil #:optional (style 'box) On 2016/01/03 23:14:22, ...
8 years, 2 months ago (2016-01-03 23:31:58 UTC) #3
pwm
doc string edit and fewer default values
8 years, 2 months ago (2016-01-04 05:17:36 UTC) #4
pwm
Addressing Harm's suggestions in patch set 2. https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/284980043/diff/1/scm/stencil.scm#newcode804 scm/stencil.scm:804: (define*-public (stencil-whiteout ...
8 years, 2 months ago (2016-01-04 05:19:20 UTC) #5
thomasmorley651
LGTM A suggestion below. Up to you, though. https://codereview.appspot.com/284980043/diff/20001/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/284980043/diff/20001/scm/stencil.scm#newcode809 scm/stencil.scm:809: and ...
8 years, 2 months ago (2016-01-06 18:24:15 UTC) #6
pwm
simplify docstring a bit
8 years, 2 months ago (2016-01-07 02:40:04 UTC) #7
pwm
Slightly simpler docstring revision. -Paul https://codereview.appspot.com/284980043/diff/20001/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/284980043/diff/20001/scm/stencil.scm#newcode809 scm/stencil.scm:809: and @code{'box}. Given @code{'outline} ...
8 years, 2 months ago (2016-01-07 02:44:51 UTC) #8
thomasmorley651
8 years, 2 months ago (2016-01-07 10:13:23 UTC) #9
Very nice, thanks.
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b