[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #12882 [Onionoo]: Logging Framework
#12882: Logging Framework
-----------------------------+--------------------------
Reporter: iwakeh | Owner: iwakeh
Type: enhancement | Status: needs_review
Priority: normal | Milestone:
Component: Onionoo | Version:
Resolution: | Keywords:
Actual Points: | Parent ID:
Points: |
-----------------------------+--------------------------
Comment (by karsten):
Hmm, I looked through the first few hundred lines of 0001-whitespace-
changes.patchâ, and those changes don't confirm to what I intended to
write above. Here are some examples:
{{{
diff --git a/src/main/java/org/torproject/onionoo/cron/Main.java
b/src/main/java/org/torproject/onionoo/cron/Main.java
index bd4b95a..3fb355f 100644
--- a/src/main/java/org/torproject/onionoo/cron/Main.java
+++ b/src/main/java/org/torproject/onionoo/cron/Main.java
@@ -26,7 +26,7 @@ public class Main {
Logger.printStatusTime("Acquired lock");
} else {
Logger.printErrorTime("Could not acquire lock. Is Onionoo "
- + "already running? Terminating");
+ + "already running? Terminating");
return;
}
@@ -68,10 +68,9 @@ public class Main {
Logger.printStatusTime("Released lock");
} else {
Logger.printErrorTime("Could not release lock. The next "
- + "execution may not start as expected");
+ + "execution may not start as expected");
}
Logger.printStatus("Terminating.");
}
}
-
}}}
The first two changes should go away, because those lines are
continuations of the previous line, and therefore should use 4 spaces, not
2. The last change should go away, because files should end with two
newlines.
{{{
diff --git
a/src/main/java/org/torproject/onionoo/docs/BandwidthDocument.java
b/src/main/java/org/torproject/onionoo/docs/BandwidthDocument.java
index ea20a5e..021c2f7 100644
--- a/src/main/java/org/torproject/onionoo/docs/BandwidthDocument.java
+++ b/src/main/java/org/torproject/onionoo/docs/BandwidthDocument.java
@@ -8,20 +8,22 @@ public class BandwidthDocument extends Document {
@SuppressWarnings("unused")
private String fingerprint;
+
public void setFingerprint(String fingerprint) {
this.fingerprint = fingerprint;
}
}}}
This additional newline shouldn't be there, because the setter is
"grouped" together with its attribute by omitting the newline. Does that
make sense, or do you prefer if we change that?
{{{
public void clearDirty() {
this.isDirty = false;
}
- private SortedMap<Long, long[]> writeHistory =
- new TreeMap<Long, long[]>();
+ private SortedMap<Long, long[]> writeHistory
+ = new TreeMap<Long, long[]>();
+
public void setWriteHistory(SortedMap<Long, long[]> writeHistory) {
this.writeHistory = writeHistory;
}
}}}
Apart from the 2 spaces which should be 4, I don't have a clear preference
whether the `=` should be on the first or second line. We can change that
if you want.
{{{
- ? endMillis : history.tailMap(startMillis).firstKey();
- if (previousEndMillis <= startMillis &&
- nextStartMillis >= endMillis) {
- history.put(startMillis, new long[] { startMillis, endMillis,
- bandwidth });
+ ? endMillis : history.tailMap(startMillis).firstKey();
+ if (previousEndMillis <= startMillis
+ && nextStartMillis >= endMillis) {
+ history.put(startMillis, new long[]{startMillis, endMillis,
+ bandwidth});
}}}
Similarly, I don't feel strongly about the `&&` being at the line end or
beginning. Happy to change that if you prefer.
What I don't like as much is the `long[]{startMillis,` part which I find
kinda hard to read.
Anyway, how about we separate the whitespace discussion from the logging
part? I can't apply your patches 0002 to 0004 without 0001. Would you be
able to rebase those patches to current master without your 0001 patch,
and then we move the reformatting discussion to a separate ticket?
Sorry this is so difficult. Thanks for your contribution!
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12882#comment:9>
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