#19607: avoid repeated keyword strings ---------------------------------+----------------------------------- Reporter: iwakeh | Owner: karsten Type: enhancement | Status: needs_revision Priority: Medium | Milestone: metrics-lib 1.7.0 Component: Metrics/metrics-lib | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: ---------------------------------+----------------------------------- Changes (by karsten):
* status: needs_review => needs_revision Comment: Replying to [comment:9 iwakeh]: > Replying to [comment:8 karsten]: > > Looks good! > > > > Just one question/suggestion: what do you think about moving `serverAtMostOnce` and other parts that are only relevant for parsing server descriptors to `ServerDescriptorImpl`? > > My reasoning was: if a new field is added in `Key` the sets for at-most, exactly, etc. are immediately in sight and the new key could be added to the appropriate set(s). But, I don't feel strongly about it; just had to make a choice while coding. Okay, in that case I'd prefer if we keep descriptor-related things in the respective descriptor classes. Both would work, but that seems potentially less confusing in the long run. > > Other than that, I'm not entirely sure what you mean by "additional lines will go away" in your comment above. Which lines do you mean? > > Sounds a little cryptic :-) The lines listed as new in the git stats. I'm referring to `protected void check*Keywords(Set<String> keywords)` methods, which could be replaced by `protected void check*Keys(Set<Key> keys)` eventually. Makes more sense! How do we proceed? Do you want to apply these changes to all descriptors, and I review those changes, or the other way around? -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19607#comment:10> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs