FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/


Description:
cmd/api: add exception file
Fixes build.

Please review this at http://codereview.appspot.com/6586074/

Affected files:
M api/README
A api/except.txt
M src/cmd/api/goapi.go
M src/run.bash


Index: api/README
===================================================================
--- a/api/README
+++ b/api/README
@@ -5,6 +5,9 @@
go1.txt (and similarly named files) are frozen once a version has been
shipped. Each file adds new lines but does not remove any.

+except.txt lists features that may disappear without breaking
+true compatibility. The only package there is text/template/parse.
+
next.txt is the only file intended to be mutated. It's a list of
features that may be added to the next version. It only affects
warning output from the go api tool.
Index: api/except.txt
===================================================================
new file mode 100644
--- /dev/null
+++ b/api/except.txt
@@ -0,0 +1,2 @@
+pkg text/template/parse, type DotNode bool
+pkg text/template/parse, type Node interface { Copy, String, Type }
Index: src/cmd/api/goapi.go
===================================================================
--- a/src/cmd/api/goapi.go
+++ b/src/cmd/api/goapi.go
@@ -38,11 +38,12 @@
var (
// TODO(bradfitz): once Go 1.1 comes out, allow the -c flag to take a
comma-separated
// list of files, rather than just one.
- checkFile = flag.String("c", "", "optional filename to check API against")
- allowNew = flag.Bool("allow_new", true, "allow API additions")
- nextFile = flag.String("next", "", "optional filename of tentative
upcoming API features for the next release. This file can be lazily
maintained. It only affects the delta warnings from the -c file printed on
success.")
- verbose = flag.Bool("v", false, "verbose debugging")
- forceCtx = flag.String("contexts", "", "optional comma-separated list of
<goos>-<goarch>[-cgo] to override default contexts.")
+ checkFile = flag.String("c", "", "optional filename to check API
against")
+ allowNew = flag.Bool("allow_new", true, "allow API additions")
+ exceptFile = flag.String("except", "", "optional filename of packages
that are allowed to change without triggering a failure in the tool")
+ nextFile = flag.String("next", "", "optional filename of tentative
upcoming API features for the next release. This file can be lazily
maintained. It only affects the delta warnings from the -c file printed on
success.")
+ verbose = flag.Bool("v", false, "verbose debugging")
+ forceCtx = flag.String("contexts", "", "optional comma-separated list
of <goos>-<goarch>[-cgo] to override default contexts.")
)

// contexts are the default contexts which are scanned, unless
@@ -198,6 +199,13 @@
}
}

+ var exception = make(map[string]bool) // exception => true
+ if *exceptFile != "" {
+ for _, feature := range fileFeatures(*exceptFile) {
+ exception[feature] = true
+ }
+ }
+
take := func(sl *[]string) string {
s := (*sl)[0]
*sl = (*sl)[1:]
@@ -207,8 +215,11 @@
for len(required) > 0 || len(features) > 0 {
switch {
case len(features) == 0 || required[0] < features[0]:
- fmt.Fprintf(bw, "-%s\n", take(&required))
- fail = true // broke compatibility
+ feature := take(&required)
+ fmt.Fprintf(bw, "-%s\n", feature)
+ if !exception[feature] {
+ fail = true // broke compatibility
+ }
case len(required) == 0 || required[0] > features[0]:
newFeature := take(&features)
if optional[newFeature] {
Index: src/run.bash
===================================================================
--- a/src/run.bash
+++ b/src/run.bash
@@ -112,7 +112,7 @@

echo
echo '# Checking API compatibility.'
-go tool api -c $GOROOT/api/go1.txt -next $GOROOT/api/next.txt
+go tool api -c $GOROOT/api/go1.txt -next $GOROOT/api/next.txt -except
$GOROOT/api/except.txt

echo
echo ALL TESTS PASSED

Search Discussions

  • Adg at Oct 4, 2012 at 12:48 am
    LGTM

    but wait for brad and others to chime in

    http://codereview.appspot.com/6586074/
  • Adg at Oct 4, 2012 at 12:48 am
    http://codereview.appspot.com/6586074/diff/1/api/README
    File api/README (right):

    http://codereview.appspot.com/6586074/diff/1/api/README#newcode8
    api/README:8: except.txt lists features that may disappear without
    breaking
    double space between that and may

    http://codereview.appspot.com/6586074/
  • Bradfitz at Oct 4, 2012 at 12:54 am
    LGTM on the code.

    Still unfortunate on going this route at all, but it's probably
    inevitable for something else anyway, so we'll need the mechanism
    regardless.



    https://codereview.appspot.com/6586074/diff/1/src/cmd/api/goapi.go
    File src/cmd/api/goapi.go (right):

    https://codereview.appspot.com/6586074/diff/1/src/cmd/api/goapi.go#newcode219
    src/cmd/api/goapi.go:219: fmt.Fprintf(bw, "-%s\n", feature)
    use different symbol failures vs permitted omissions?

    I could see this being confusing if there's another minus failure and we
    can't visually see which is the real failure.

    Not a big deal, though.

    https://codereview.appspot.com/6586074/
  • Rob Pike at Oct 4, 2012 at 12:59 am
    I had the same thought but couldn't think of a symbol. Suggest one and
    I'll use it.

    -rob
  • David Symonds at Oct 4, 2012 at 1:00 am

    On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike wrote:

    I had the same thought but couldn't think of a symbol. Suggest one and
    I'll use it.
    U+1F4A9
  • Dave Cheney at Oct 4, 2012 at 1:12 am

    On Thu, Oct 4, 2012 at 11:00 AM, David Symonds wrote:
    On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike wrote:

    I had the same thought but couldn't think of a symbol. Suggest one and
    I'll use it.
    U+1F4A9
  • Brad Fitzpatrick at Oct 4, 2012 at 1:15 am
    As much as I loves me an interrobang, it doesn't really work here. It
    suggests anger-and-confusion. We want to express misfortune/sadness.
    On Wed, Oct 3, 2012 at 6:11 PM, Dave Cheney wrote:


    On Thu, Oct 4, 2012 at 11:00 AM, David Symonds wrote:
    On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike wrote:

    I had the same thought but couldn't think of a symbol. Suggest one and
    I'll use it.
    U+1F4A9
  • Brad Fitzpatrick at Oct 4, 2012 at 1:08 am

    On Wed, Oct 3, 2012 at 5:59 PM, Rob Pike wrote:

    I had the same thought but couldn't think of a symbol. Suggest one and
    I'll use it.
    ~ ? Looks like a minus, and also looks kinda unhappy.
  • R at Oct 4, 2012 at 1:35 am
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=43a4b23b65a3 ***

    cmd/api: add exception file
    Fixes build.

    R=golang-dev, adg, bradfitz, dsymonds, dave
    CC=golang-dev
    http://codereview.appspot.com/6586074


    http://codereview.appspot.com/6586074/
  • Russ Cox at Oct 5, 2012 at 9:33 pm
    LGTM

    However:

    Please add an unexported method to package parse's type Node, so that
    future extensions are guaranteed safe (because all the implementations
    are in package parse) instead of just presumed safe.

    Are we really sure about DotNode? It might be better to leave
    DotNode's definition but mark it as unused and introduce a separate
    DotNodePos with an embedded DotNode and Pos. What if someone is
    rewriting the parse tree and creating a DotNode(false) somewhere?
  • Rob Pike at Oct 5, 2012 at 9:42 pm

    On Sat, Oct 6, 2012 at 7:25 AM, Russ Cox wrote:
    LGTM

    However:

    Please add an unexported method to package parse's type Node, so that
    future extensions are guaranteed safe (because all the implementations
    are in package parse) instead of just presumed safe.
    OK; CL in prep.
    Are we really sure about DotNode? It might be better to leave
    DotNode's definition but mark it as unused and introduce a separate
    DotNodePos with an embedded DotNode and Pos. What if someone is
    rewriting the parse tree and creating a DotNode(false) somewhere?
    I'm comfortable. The package doc says, don't use this.

    -rob

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 4, '12 at 12:45a
activeOct 5, '12 at 9:42p
posts12
users6
websitegolang.org

People

Translate

site design / logo © 2021 Grokbase