| Message ID | 20230207073607.913994-6-juerg.haefliger@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | annotations: Cleanups and splitting | expand |
On Tue, Feb 07, 2023 at 08:36:07AM +0100, Juerg Haefliger wrote: > When writing the annotations file, separate them into two groups: With > and without a note. Write the group with notes first and separate the > other group with a visual marker. > > The idea is that all configs that are set/modified manually should have > an annotation note and putting them at the top of the annotations file > should make it easier to figure out what the config of this kernel is > about. I'm wondering if we should move the configs-with-note group at the end of the file to make sure that we always prioritize configs-with-note over configs-without-note, so that we're not tempted to add stuff at the top that can be potentially overridden by the same config rule defined later in the same file (typically this shouldn't happen if we search for the config that we want to change, but I think the config-with-note at the end is less bug prone). If the reason is a better readability we can still provide an annotations command to query/show only configs with a note. What do you think? Thanks, -Andrea
On Wed, 8 Feb 2023 07:57:40 +0100 Andrea Righi <andrea.righi@canonical.com> wrote: > On Tue, Feb 07, 2023 at 08:36:07AM +0100, Juerg Haefliger wrote: > > When writing the annotations file, separate them into two groups: With > > and without a note. Write the group with notes first and separate the > > other group with a visual marker. > > > > The idea is that all configs that are set/modified manually should have > > an annotation note and putting them at the top of the annotations file > > should make it easier to figure out what the config of this kernel is > > about. > > I'm wondering if we should move the configs-with-note group at the end of > the file to make sure that we always prioritize configs-with-note over > configs-without-note, so that we're not tempted to add stuff at the top > that can be potentially overridden by the same config rule defined later > in the same file (typically this shouldn't happen if we search for the > config that we want to change, but I think the config-with-note at the > end is less bug prone). Policies with notes are manually added and important so should be a the top IMO. Otherwise they get (visually) lost at the end after 1000s of mechanically added policies. And policies with notes should take precedence over ones without notes, no? So that way we can't loose them. ...Juerg > If the reason is a better readability we can still provide an > annotations command to query/show only configs with a note. > > What do you think? > > Thanks, > -Andrea
On Wed, Feb 08, 2023 at 08:36:09AM +0100, Juerg Haefliger wrote: > On Wed, 8 Feb 2023 07:57:40 +0100 > Andrea Righi <andrea.righi@canonical.com> wrote: > > > On Tue, Feb 07, 2023 at 08:36:07AM +0100, Juerg Haefliger wrote: > > > When writing the annotations file, separate them into two groups: With > > > and without a note. Write the group with notes first and separate the > > > other group with a visual marker. > > > > > > The idea is that all configs that are set/modified manually should have > > > an annotation note and putting them at the top of the annotations file > > > should make it easier to figure out what the config of this kernel is > > > about. > > > > I'm wondering if we should move the configs-with-note group at the end of > > the file to make sure that we always prioritize configs-with-note over > > configs-without-note, so that we're not tempted to add stuff at the top > > that can be potentially overridden by the same config rule defined later > > in the same file (typically this shouldn't happen if we search for the > > config that we want to change, but I think the config-with-note at the > > end is less bug prone). > > Policies with notes are manually added and important so should be a the top > IMO. Otherwise they get (visually) lost at the end after 1000s of mechanically > added policies. And policies with notes should take precedence over ones > without notes, no? So that way we can't loose them. OK, yeah at the end if shouldn't be a problem, because we always run an updateconfigs that will take care of removing duplicate lines, so in practice there's no risk of potential overrides and having the configs-with-note at the top is better in terms of readability. Alright, in general I like all these changes/cleanups, thanks for doing all of this Juerg! -Andrea
diff --git a/debian/scripts/misc/kconfig/annotations.py b/debian/scripts/misc/kconfig/annotations.py index 03b01baad46b..113ce53eb4e0 100644 --- a/debian/scripts/misc/kconfig/annotations.py +++ b/debian/scripts/misc/kconfig/annotations.py @@ -262,6 +262,17 @@ class Annotation(Config): if not self.config[conf]['policy']: del self.config[conf] + def _sorted(self, config): + """ Sort configs alphabetically but return configs with a note first """ + w_note = [] + wo_note = [] + for c in sorted(config): + if 'note' in config[c]: + w_note.append(c) + else: + wo_note.append(c) + return w_note + wo_note + def save(self, fname: str): """ Save annotations data to the annotation file """ # Compact annotations structure @@ -284,7 +295,8 @@ class Annotation(Config): tmp_a = Annotation(fname) # Only save local differences (preserve includes) - for conf in sorted(self.config): + marker = False + for conf in self._sorted(self.config): new_val = self.config[conf] if 'policy' not in new_val: continue @@ -307,6 +319,11 @@ class Annotation(Config): # Separate policy and note lines, # followed by an empty line line += f'\n{conf : <47} note<{val}>\n' + elif not marker: + # Write out a marker indicating the start of annotations + # without notes + tmp.write('\n# ---- Annotations without notes ----\n\n') + marker = True tmp.write(line + "\n") # Replace annotations with the updated version
When writing the annotations file, separate them into two groups: With and without a note. Write the group with notes first and separate the other group with a visual marker. The idea is that all configs that are set/modified manually should have an annotation note and putting them at the top of the annotations file should make it easier to figure out what the config of this kernel is about. Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> --- debian/scripts/misc/kconfig/annotations.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)