Skip to content

Conversation

@ArneBab
Copy link
Contributor

@ArneBab ArneBab commented Jul 29, 2020

No description provided.

Debora Wöpcke and others added 30 commits December 19, 2015 12:39
Fixed indentation (and obvious bug because of it).
Removed some System.out logging, fixed other.
Doing the first refacturing.

--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
Logging is left commented out for when another logging mechanism is
eventually added.

--HG--
branch : eclipse-separation
Logging was commented out.

--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
…gged time.

--HG--
branch : eclipse-separation
…o be read.

--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
Moved Priority.

--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
--HG--
branch : eclipse-separation
@ArneBab ArneBab mentioned this pull request Jul 29, 2020
Copy link
Collaborator

@desyncr desyncr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far. Mentioned a couple of places where there's either blocks of code commented or unnecessary files added. Haven't yet arrived at the interesting parts.

One thing to note is the uploader. Can it be moved to another repository? Or it's too coupled with plugin-library? It also would make the reviewer a bit easier.

package plugins.Library.util;

import freenet.support.Fields; // JDK6: use this instead of Arrays.binarySearch
// import freenet.support.Fields; // JDK6: use this instead of Arrays.binarySearch
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary comment.

throw new IllegalStateException("More parts done than known");
}
pdone++;
// System.err.println("DEBUG: " + this + " done " + pdone + "/" + known);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watch out these left out comments. Also in line 120, 127.

@@ -0,0 +1,7 @@
build
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this file? The main repository is a git repository not a mercurial (AFAIK).

@@ -0,0 +1,7 @@
#!/bin/sh -x
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file for local tests? Can we clarify it if so?

private static int version = 36;
public static final String plugName = "Library " + getVersion();


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple empty lines.

/** The uploaded index on Freenet. This never changes, it just gets updated. */
ProtoIndex idxFreenet;

//<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out block of code.

// uri = hlsc.insert(ib, false, null, priorityClass, ctx);
// if (progress != null)
// progress.addPartKnown(0, true);
//<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out code.

}

}
//<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out code.

static {
Logger.registerClass(Packer.class);
}
// static {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out code.

// read local copy of aggression
int agg = getAggression();
if(logDEBUG) Logger.debug(this, "Aggression = "+agg+" tasks size = "+tasks.size());
// if(logDEBUG) Logger.debug(this, "Aggression = "+agg+" tasks size = "+tasks.size());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out code.

public interface ArchiverFactory {
<T, S extends ObjectStreamWriter & ObjectStreamReader>
LiveArchiver<T, SimpleProgress>
newArchiver(S rw, String mime, int size,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Methods in interfaces should have at least rudimentary documentation, otherwise nobody will ever know what that int is meant to be…

return edition-1;
FileInputStream fis = null;
try {
fis = new FileInputStream(new File(SpiderIndexUploader.EDITION_FILENAME));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use try-with-resources.

/** The uploaded index on Freenet. This never changes, it just gets updated. */
ProtoIndex idxFreenet;

//<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is completely inacceptible.

@Bombe
Copy link

Bombe commented Jul 30, 2020

This PR is way too large and in rather bad shape. Are there smaller PRs this is based upon?

@desyncr
Copy link
Collaborator

desyncr commented Jul 31, 2020

@Bombe There are various branches with diverse changes. I believe this one are the most portable changes we can PR and review. These changes contain the SnakeYAML upgrade.

@desyncr desyncr mentioned this pull request Jul 31, 2020
@ArneBab ArneBab mentioned this pull request Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants