Code review comment for lp:~axwalk/juju-core/lp1121916-juju-status-filters

Revision history for this message
Roger Peppe (rogpeppe) wrote :

still LGTM with two final thoughts.

https://codereview.appspot.com/12235043/diff/42001/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/12235043/diff/42001/cmd/juju/status.go#newcode137
cmd/juju/status.go:137: const valid =
"abcdefghijklmnopqrstuvwxyz0123456789-*"
or:

var validPattern = regexp.MustCompile("^[a-z0-9-*]$")

and use if !validPattern.MatchString(s) instead of validatePattern.

which would make it easier to add extra stuff if, for instance,
we started to allow unicode characters in service names.

https://codereview.appspot.com/12235043/diff/42001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/12235043/diff/42001/state/unit.go#newcode96
state/unit.go:96: var _ AgentEntity = (*Unit)(nil)
This has moved into interface.go now.

https://codereview.appspot.com/12235043/

« Back to merge proposal