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).

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.
>>
>
>