#29166: Run modules from Java only --------------------------------+------------------------------ Reporter: karsten | Owner: karsten Type: enhancement | Status: needs_review Priority: Medium | Milestone: Component: Metrics/Statistics | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: irl | Sponsor: --------------------------------+------------------------------
Comment (by karsten): Replying to [comment:4 notirl]: > {{{ > this.connection = DriverManager.getConnection(jdbcString); > }}} > > Should this not still be explicitly `this.jdbcString`? It's still a class member and not a local variable. It's indeed a class variable now, not an instance variable anymore, so we cannot use `this.` anymore. Of course, we could argue whether an instance variable might still make more sense. I didn't pay too much attention to this, because I'm planning to simplify code around database access across modules very soon. > Some of the paths have been defined with `/` in them, which breaks compatibility with non-Unix operating systems. (Windows uses `\` and RISC OS uses `:` as examples.) I think we don't care but it's worth being aware of it. > > We should have a policy and be explicit about our use of, or avoidance of, java.nio.file.Paths. I agree that using `Paths` would be cleaner. I tried to keep `Paths` but then realized that it's producing fewer code changes to just switch (back) to `File` in some places. Again, this is code where I anticipate major changes in the near future. For example, I'd like to have a single place in the code that reads descriptors and passes them to the modules, much like how Onionoo works. That's going to eliminate a lot of places where we had previously used `Paths`. > In general this looks OK, probably is best to not deploy it just before Brussels though. I can give it a more thorough review after Brussels. I think that not deploying before Brussels is a fine plan. Maybe we can talk about the review ''in'' Brussels and also do the deployment together. Thanks for this initial review! -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/29166#comment:5> 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