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