The internal Protocol Buffer style guide actually forbid it.  :-P


2013/7/26 juliusv <notifications@github.com>

> In config/config.proto:
>
> > +// You may obtain a copy of the License at
> > +//
> > +// http://www.apache.org/licenses/LICENSE-2.0
> > +//
> > +// Unless required by applicable law or agreed to in writing, software
> > +// distributed under the License is distributed on an "AS IS" BASIS,
> > +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > +// See the License for the specific language governing permissions and
> > +// limitations under the License.
> > +
> > +package io.prometheus.alert_manager;
> > +
> > +// A regex-based label filter used in aggregations.
> > +message Filter {
> > +  // The regex matching the label name.
> > +  required string name_re = 1;
>
> Heh, from https://developers.google.com/protocol-buffers/docs/proto: Some
> engineers at Google have come to the conclusion that using required does
> more harm than good; they prefer to use only optional and repeated.
> However, this view is not universal. I'll keep it for now, since in this
> case I think it does more good than harm (better to get a line number than
> saying "hey user, somewhere in this huge file, there is a "name_re"
> missing).
> … <#1401b593f3fd6bc5_>
> On Fri, Jul 26, 2013 at 3:55 PM, Julius Volz <julius@soundcloud.com>
> wrote: > Actually thinking again, the error message is better than any I
> could > deliver manually, since it includes the line number. It would
> cumbersome to > point the user to the erroneous location without that. > >
> > On Fri, Jul 26, 2013 at 3:54 PM, Julius Volz <julius@soundcloud.com>wrote:
> > >> On Fri, Jul 26, 2013 at 3:25 PM, Matt T. Proud <
> notifications@github.com>wrote: > >> In config/config.proto: >> >> > +//
> You may obtain a copy of the License at >> > +// >> > +//
> http://www.apache.org/licenses/LICENSE-2.0 >> > +// >> > +// Unless
> required by applicable law or agreed to in writing, software >> > +//
> distributed under the License is distributed on an "AS IS" BASIS, >> > +//
> WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>
> > +// See the License for the specific language governing permissions and
> >> > +// limitations under the License. >> > + >> > +package
> io.prometheus.alert_manager; >> > + >> > +// A regex-based label filter
> used in aggregations. >> > +message Filter { >> > + // The regex matching
> the label name. >> > + required string name_re = 1; >> >> Kenton Varda, the
> author of Protocol Buffers, ended up saying in a >> blogpost a while back
> that required was a feature he wishes he could >> take back from the stack.
> >> > Would like to read that at some point, couldn't find it immediately. >
> Distrust authority without proper reasoning! ;) > >> Plus, the validations
> that you get are incredibly unfriendly if a >> required field is missing,
> which means that the user wouldn't really be >> able to act on an error
> message from the deserializer. >> >> The reported error looks like: > >
> line 8: message io_prometheus_alert_manager.Filter missing required > field
> "name_re" > > Looks pretty readable to me. > > I'll change it, but I
> suspect this discussion will come up again in the > future in other config
> files, and at some point I'd like to have better > reasoning for doing
> things one way or the other. >
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/prometheus/alert_manager/pull/7/files#r5423348>
> .
>



-- 
Key: 0xC42234343FDE43A8
Fingerprint: 6945 9248 4362 68CB EA83  F1EA C422 3434 3FDE 43A8