[PATCH] D137091: [llvm-readobj] Rename JSON Flag fields to be more consistent

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 24 00:29:41 PST 2023


jhenderson added a comment.

In D137091#4147838 <https://reviews.llvm.org/D137091#4147838>, @paulkirth wrote:

> In D137091#4146674 <https://reviews.llvm.org/D137091#4146674>, @jhenderson wrote:
>
>> LGTM, with one caveat: has there been an LLVM release which produced the old name in a usable form? If so, this probably needs a release note. If on the other hand, the JSON would have been invalid, or the name wasn't emitted at all, there's no need to change anything.
>
> I'm not sure. I'm almost positive that we always emit something fundamentally broken when we emit anything that contains `Flags`.  Still it's probably best to just add the release note, and err on the side of caution, so I'll add that when I get a chance later today.

You might want to make the release note more general. Something along the lines of "Made significant changes to JSON output format of llvm-readobj/llvm-readelf to improve correctness and clarity." That way, it'll cover all the other issues too.

>> Sorry for taking so long to get back around to this patch series. Is there a particular one you'd like me to look at as a priority? I'll otherwise aim to do 1 or 2 a day, depending on my other work and review load.
>
> I guess bottom of the stack to the top is preferable, just to make landing easier, but other than that I don't think there is much preference.

No problem, will do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137091/new/

https://reviews.llvm.org/D137091



More information about the llvm-commits mailing list