[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file
#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods
for loading and saving a history file
---------------------------------+-----------------------------------
Reporter: karsten | Owner: karsten
Type: enhancement | Status: new
Priority: Medium | Milestone: metrics-lib 1.6.0
Component: Metrics/metrics-lib | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
---------------------------------+-----------------------------------
Changes (by karsten):
* status: needs_review => new
Comment:
Replying to [comment:3 iwakeh]:
> Please find a [https://gitweb.torproject.org/user/iwakeh/metrics-
lib.git/commit/?h=task-20521-2&id=c8707e56904119fc072b700b9f839ac371c1d83e
commit] on top of your branch with some modernization (reduced number of
lines) and a checkstyle complaint removed.
Oops, I totally missed that checkstyle complaint. But I think your fix
only removes the complaint and lets Javadoc think that the `@deprecated`
line is yet another paragraph, which is not what we want. Indenting the
two lines below that line is the better fix. Removed that part from your
commit and added a fixup commit to remove the checkstyle warning.
I also found that you're using `Files` methods that are introduced in Java
8, but we're still at Java 7. Well, Eclipse found that for me. I added
another fixup commit for those. Can you configure your IDE to use the
Java 7 SE library? (But okay, I won't say anything after complaining
about unresolved checkstyle warnings in other patches and then overlooking
one myself, heh.)
Please find [https://gitweb.torproject.org/user/karsten/metrics-
lib.git/log/?h=task-20521 my updated task-20521 branch].
> Wouldn't this be the right time to add a test class for
`DescriptorReader`(Impl)?
Hmmmmmmmm, yes. Setting this ticket to new and not merging yet until we
have such a test class.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20521#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs