Code review comment for lp://staging/~dimitern/juju-core/038-upgrade-charm-switch

nearly there, last round I think

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (left):

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go#oldcode100
cmd/juju/upgradecharm.go:100: } else if _, bumpRevision =
ch.(*charm.Dir); !bumpRevision {
...here -- in which case we should only set bumpRevision if
explicitRevision is false.

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go#newcode54
cmd/juju/upgradecharm.go:54: The new charm may add new relations and
configuration settings.
I think we can drop this line.

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go#newcode145
cmd/juju/upgradecharm.go:145: if *newURL == *oldURL && !explicitRevision
{
If newURL matches oldURL, we have to take this branch. We don't need to
worry about explicitRevision until...

https://codereview.appspot.com/8540050/diff/32001/cmd/juju/upgradecharm.go#newcode162
cmd/juju/upgradecharm.go:162: sch, err := conn.PutCharm(newURL, repo,
bumpRevision)
It's somewhat crazy that we can specify a URL with an additional param
that means "lol not really". But that's way out of scope.

https://codereview.appspot.com/8540050/