| Message ID | bf8cc342-52aa-44ee-8bce-ce2be6406904@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.20] public/version: soften wording for deprecated sub-ops | expand |
On 06/01/2025 11:04 am, Jan Beulich wrote: > These interfaces were - afaict - originally introduced this way on the > firm assumption that the used array sizes would be good virtually > forever. While this assumption turned out to not be true for at least > some of them, this still doesn't really render them "broken": They still > fit their original purpose, and they are still usable for a fair subset > of environments. Re-word the comments accordingly. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> No. The community voted and rejected this opinion. ~Andrew
On 06.01.2025 12:08, Andrew Cooper wrote: > On 06/01/2025 11:04 am, Jan Beulich wrote: >> These interfaces were - afaict - originally introduced this way on the >> firm assumption that the used array sizes would be good virtually >> forever. While this assumption turned out to not be true for at least >> some of them, this still doesn't really render them "broken": They still >> fit their original purpose, and they are still usable for a fair subset >> of environments. Re-word the comments accordingly. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > No. > > The community voted and rejected this opinion. That's not my recollection of what was voted on, and with the vote results not being available referring to them is unhelpful anyway. My (admittedly vague) recollection is that it was decided to leave enough room for wording choice by submitters. That would cover your original patch, and it would equally cover mine. Jan
On Mon, 6 Jan 2025, Jan Beulich wrote: > On 06.01.2025 12:08, Andrew Cooper wrote: > > On 06/01/2025 11:04 am, Jan Beulich wrote: > >> These interfaces were - afaict - originally introduced this way on the > >> firm assumption that the used array sizes would be good virtually > >> forever. While this assumption turned out to not be true for at least > >> some of them, this still doesn't really render them "broken": They still > >> fit their original purpose, and they are still usable for a fair subset > >> of environments. Re-word the comments accordingly. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > No. > > > > The community voted and rejected this opinion. > > That's not my recollection of what was voted on, and with the vote results > not being available referring to them is unhelpful anyway. > > My (admittedly vague) recollection is that it was decided to leave enough > room for wording choice by submitters. That would cover your original > patch, and it would equally cover mine. The community-wide survey indicated that it is acceptable to use the term "broken" in our documentation [1]. While the survey was not tied to a specific instance, it was undoubtedly influenced by the ongoing discussion at the time. If the purpose of this patch is to replace the term "broken", as it would seem from the commit message, I would recommend dropping the patch and leaving the wording as it is, given that the community has expressed approval of its use. Let us respect that decision. However, if the goal is to improve clarity by specifying "due to its size limitations" and noting that the truncation occurs "silently", then I believe the patch could be reviewed with that objective in mind. In other words, we should not replace "broken" simply for the sake of doing so. That discussion has already been settled. When reviewing this patch, our focus should be on its other merits, if any. Based on the above, I would not take the patch in its current form. But if Jan is up for rewording the commit message, and focusing purely on the clarity of the in-code comments maybe a future version could be acceptable. [1] https://cryptpad.fr/form/#/2/form/view/7ByH95Vd7KiDOvN4wjV5iUGlMuZbkVdwk7cYpZdluWo/
On 06.01.2025 23:01, Stefano Stabellini wrote: > On Mon, 6 Jan 2025, Jan Beulich wrote: >> On 06.01.2025 12:08, Andrew Cooper wrote: >>> On 06/01/2025 11:04 am, Jan Beulich wrote: >>>> These interfaces were - afaict - originally introduced this way on the >>>> firm assumption that the used array sizes would be good virtually >>>> forever. While this assumption turned out to not be true for at least >>>> some of them, this still doesn't really render them "broken": They still >>>> fit their original purpose, and they are still usable for a fair subset >>>> of environments. Re-word the comments accordingly. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> No. >>> >>> The community voted and rejected this opinion. >> >> That's not my recollection of what was voted on, and with the vote results >> not being available referring to them is unhelpful anyway. >> >> My (admittedly vague) recollection is that it was decided to leave enough >> room for wording choice by submitters. That would cover your original >> patch, and it would equally cover mine. > > The community-wide survey indicated that it is acceptable to use the > term "broken" in our documentation [1]. While the survey was not tied to > a specific instance, it was undoubtedly influenced by the ongoing > discussion at the time. IOW this re-confirms (to me at least) that the vote in itself was ambiguous. I have no issue at all with the use of the word "broken" in documentation or code comments, provided this accurately describes the situation. Which it doesn't here. > If the purpose of this patch is to replace the term "broken", as it > would seem from the commit message, I would recommend dropping the patch > and leaving the wording as it is, given that the community has expressed > approval of its use. Let us respect that decision. > > However, if the goal is to improve clarity by specifying "due to its > size limitations" and noting that the truncation occurs "silently", then > I believe the patch could be reviewed with that objective in mind. > > In other words, we should not replace "broken" simply for the sake of > doing so. That discussion has already been settled. When reviewing this > patch, our focus should be on its other merits, if any. > > Based on the above, I would not take the patch in its current form. But > if Jan is up for rewording the commit message, and focusing purely on > the clarity of the in-code comments maybe a future version could be > acceptable. Assuming the above doesn't change your view, and assuming no-one else is going to express views in favor of the wording change, I'll consider the patch rejected. And I'll be once again left with the impression that things are treated neither equally nor objectively in situations like this one: To get one's perspective through unaltered one only needs to resist hard enough to any attempt to find a middle ground. That's not a good environment to work in, and not something I'd call a "community". Jan > [1] https://cryptpad.fr/form/#/2/form/view/7ByH95Vd7KiDOvN4wjV5iUGlMuZbkVdwk7cYpZdluWo/
On Tue, Jan 07, 2025 at 09:32:05AM +0100, Jan Beulich wrote: > On 06.01.2025 23:01, Stefano Stabellini wrote: > > On Mon, 6 Jan 2025, Jan Beulich wrote: > >> On 06.01.2025 12:08, Andrew Cooper wrote: > >>> On 06/01/2025 11:04 am, Jan Beulich wrote: > >>>> These interfaces were - afaict - originally introduced this way on the > >>>> firm assumption that the used array sizes would be good virtually > >>>> forever. While this assumption turned out to not be true for at least > >>>> some of them, this still doesn't really render them "broken": They still > >>>> fit their original purpose, and they are still usable for a fair subset > >>>> of environments. Re-word the comments accordingly. > >>>> > >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >>> > >>> No. > >>> > >>> The community voted and rejected this opinion. > >> > >> That's not my recollection of what was voted on, and with the vote results > >> not being available referring to them is unhelpful anyway. > >> > >> My (admittedly vague) recollection is that it was decided to leave enough > >> room for wording choice by submitters. That would cover your original > >> patch, and it would equally cover mine. > > > > The community-wide survey indicated that it is acceptable to use the > > term "broken" in our documentation [1]. While the survey was not tied to > > a specific instance, it was undoubtedly influenced by the ongoing > > discussion at the time. > > IOW this re-confirms (to me at least) that the vote in itself was ambiguous. > I have no issue at all with the use of the word "broken" in documentation or > code comments, provided this accurately describes the situation. Which it > doesn't here. I agree with you, I don't think banning the word "broken" from documentation or code comments is helpful or desirable. I think the survey wasn't helpful: if we wanted to solve the issue around the usage of "broken" in that specific patch series, we have mechanisms to do so: calling a explicit committers vote on the specific issue. A generic survey about whether using "broken" is acceptable or not doesn't solve the specific issue of whether using "broken" in that context was accurate or not. > > If the purpose of this patch is to replace the term "broken", as it > > would seem from the commit message, I would recommend dropping the patch > > and leaving the wording as it is, given that the community has expressed > > approval of its use. Let us respect that decision. > > > > However, if the goal is to improve clarity by specifying "due to its > > size limitations" and noting that the truncation occurs "silently", then > > I believe the patch could be reviewed with that objective in mind. > > > > In other words, we should not replace "broken" simply for the sake of > > doing so. That discussion has already been settled. When reviewing this > > patch, our focus should be on its other merits, if any. > > > > Based on the above, I would not take the patch in its current form. But > > if Jan is up for rewording the commit message, and focusing purely on > > the clarity of the in-code comments maybe a future version could be > > acceptable. > > Assuming the above doesn't change your view, and assuming no-one else is > going to express views in favor of the wording change, I'll consider the > patch rejected. And I'll be once again left with the impression that > things are treated neither equally nor objectively in situations like this > one: To get one's perspective through unaltered one only needs to resist > hard enough to any attempt to find a middle ground. That's not a good > environment to work in, and not something I'd call a "community". I don't mind that much whether "broken" or "deprecated" is used, IMO I find it a matter of taste in this case, and I would leave that to the author of the patch. I can however understand your frustration with the original survey and how the results seem to extend past what the questions asked for. As said above, our community has meanings to resolve disputes around this kind of issues in the governance documents, however a public anonymous survey is not one of them AFAIK. Regards, Roger.
--- a/xen/include/public/version.h +++ b/xen/include/public/version.h @@ -22,7 +22,8 @@ /* * arg == xen_extraversion_t. * - * This API/ABI is broken. Use XENVER_extraversion2 where possible. + * This API/ABI is deprecated, for its size limitation. Use + * XENVER_extraversion2 where possible. */ #define XENVER_extraversion 1 typedef char xen_extraversion_t[16]; @@ -31,7 +32,8 @@ typedef char xen_extraversion_t[16]; /* * arg == xen_compile_info_t. * - * This API/ABI is broken and truncates data. + * This API/ABI is deprecated, for its size limitations. It may in particular + * silently truncate data. */ #define XENVER_compile_info 2 struct xen_compile_info { @@ -45,7 +47,8 @@ typedef struct xen_compile_info xen_comp /* * arg == xen_capabilities_info_t. * - * This API/ABI is broken. Use XENVER_capabilities2 where possible. + * This API/ABI is deprecated, for its size limitation. Use + * XENVER_capabilities2 where possible. */ #define XENVER_capabilities 3 typedef char xen_capabilities_info_t[1024]; @@ -54,7 +57,8 @@ typedef char xen_capabilities_info_t[102 /* * arg == xen_changeset_info_t. * - * This API/ABI is broken. Use XENVER_changeset2 where possible. + * This API/ABI is deprecated, for its size limitation. Use + * XENVER_changeset2 where possible. */ #define XENVER_changeset 4 typedef char xen_changeset_info_t[64]; @@ -116,7 +120,8 @@ typedef struct xen_feature_info xen_feat /* * arg == xen_commandline_t. * - * This API/ABI is broken. Use XENVER_commandline2 where possible. + * This API/ABI is deprecated, for its size limitation. Use + * XENVER_commandline2 where possible. */ #define XENVER_commandline 9 typedef char xen_commandline_t[1024];
These interfaces were - afaict - originally introduced this way on the firm assumption that the used array sizes would be good virtually forever. While this assumption turned out to not be true for at least some of them, this still doesn't really render them "broken": They still fit their original purpose, and they are still usable for a fair subset of environments. Re-word the comments accordingly. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Despite having looked at the series back when it was posted / discussed, I'm (now) somewhat puzzled that XENVER_compile_info didn't gain a non- truncating replacement sub-op. The commit's description doesn't say anything in this regard; it rather gives the impression that all sub-ops with limitations would gain unrestricted counterparts.