Ticket #43938

generate_packets.py: Make internal parameters argparse-controllable

Eröffnet am: 2022-02-20 11:42 Letztes Update: 2022-03-01 19:58

Auswertung:
Verantwortlicher:
Typ:
Status:
Geschlossen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Gefixt
Datei:
1

Details

Follow-up to #43931. Part of #43927.

There are currently a number of internal parameters (generate_stats, generate_logs/use_log_macro, fold_bool_into_header and lazy_overwrite) that are set in the code. They should be set depending on optional command line arguments instead.

Ticket-Verlauf (3/9 Historien)

2022-02-20 11:42 Aktualisiert von: alienvalkyrie
  • New Ticket "generate_packets.py: Make internal parameters argparse-controllable" created
2022-02-25 09:21 Aktualisiert von: alienvalkyrie
  • Verantwortlicher Update from (Keine) to alienvalkyrie
Kommentar

While testing this, I noticed that the generate_stats option doesn't work ~> #43979.

Given that the scope of this ticket is making the options command-line-controllable, not ensuring they work, I'll keep the option in anyway.

2022-02-25 09:55 Aktualisiert von: alienvalkyrie
  • Lösung Update from Keine to Accepted
Kommentar

As mentioned above; this patch exposes options with no regard for whether they actually work – the -s, --gen-stats option does not (#43979); the -B, --no-fold-bool option successfully generates code, but I haven't (yet) tested whether that code works (and if it doesn't, that has no bearing on this ticket). The other options (verbose, logs, lazy/force) are tested.

2022-02-25 11:45 Aktualisiert von: cazfi
Kommentar

Reply To alienvalkyrie

As mentioned above; this patch exposes options with no regard for whether they actually work – the -s, --gen-stats option does not (#43979)

This patch makes the situation worse in that you expose the crash, and make the code in question more officially supported. Before it was not crashing unless you modified the code. Resolve #43979 first.

2022-02-25 20:41 Aktualisiert von: alienvalkyrie
Kommentar

Reply To cazfi

and make the code in question more officially supported

I'm not sure I agree with that part; even before, it had configuration characterstics, and there were other parts of the code (comments in packets[_json].c) that explicitly referred to changing it.

Either way, this is likely gonna delay this patch for quite a while, 'cause I'll have to figure out how the stats generation was supposed to work (which might mean digging through mailing list archives for a version of the patch when it did).

2022-02-25 20:52 Aktualisiert von: cazfi
Kommentar

Reply To alienvalkyrie

Either way, this is likely gonna delay this patch for quite a while, 'cause I'll have to figure out how the stats generation was supposed to work (which might mean digging through mailing list archives for a version of the patch when it did).

Given that apparently nobody has ever even wanted to use it, I wouldn't necessarily be against resolving the issue by just dropping the feature. Replacing it with a completely new stats generation implementation would definitely be ok. Can also go with the combination of these -> drop now, but open a ticket for a future implementation.

2022-02-27 00:04 Aktualisiert von: alienvalkyrie
Kommentar

Turns out the fix for #43979 was rather simple, so I'm currently keeping it in. It also doesn't require any change to this patch, so I'll merge it as-is along with that.

2022-03-01 19:58 Aktualisiert von: alienvalkyrie
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt

Bearbeiten

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Anmelden