On 23/11/2023 16:11, Ceclan Dumitru wrote: > > > On 11/23/23 16:26, Krzysztof Kozlowski wrote: >> On 23/11/2023 15:02, mitrutzceclan wrote: >>> + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: >> >> Drop "Bindings for" and instead describe hardware. >> > > Okay > > ... > >>> + avdd-supply: >>> + description: avdd supply, can be used as reference for conversion. >>> + >>> + required: >> >> Please test your code before sending. You ignored my comment. This has >> both wrong indentation and wrong placement - should be after all >> properties and patternProperties. >> >> Do not ignore comments but respond to them. >> > > There were no errors while testing the yaml binding (with > DT_CHECKER_FLAGS=-m dt_binding_check - to make sure that this is how > bindings should be tested). Indeed I did not test the yaml if the > required properties are missing from the example. What is indicative in > this patch that it was not tested? Then your testing method might miss something, because as you can see - Rob's bot found the issue. > > I did not ignore your comment. I did not have questions about it. I > missed the indentation. Sorry about that. > > But about the placement of 'required': the example-schema does not have > the exact case of pattern properties. Also, there are multiple iio/adc > (ad4130, ad7124, ad7292) bindings that place required before > patternProperties. I assumed that this placement is correct. > > Will move it in next version. > > In regards to responding to comments: if there are no questions about a > comment and will fix in next version, should there be a response anyway > just confirming it? The point is that code did not change here and there was no Ack/Done/something reply. Best regards, Krzysztof
