[jira] [Reopened] (REEF-401) Triage TODO comments in Java code

["Brian Cho (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/REEF-401?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brian Cho reopened REEF-401:
----------------------------

> Triage TODO comments in Java code
> ---------------------------------
>
>                 Key: REEF-401
>                 URL: https://issues.apache.org/jira/browse/REEF-401
>             Project: REEF
>          Issue Type: Sub-task
>          Components: All
>            Reporter: Mariia Mykhailova
>            Assignee: Mariia Mykhailova
>            Priority: Minor
>
> TodoComment check reports comments which contain a predefined pattern ("TODO")
> To fix the violations, we need two steps:
> 1. Go through existing TODO comments and triage them; create and link JIRA bugs/tasks for ones which we intend to fix and delete the ones we don't
> 2. Update checkstyle configuration to ignore comments starting with "TODO (JIRA ...)", so that the violation is reported only for untriaged TODOs
> There is a bunch of "TODO: Document" comments, which typically overlap with JavadocType (missing comment) violations - I'll handle them together with javadoc issues in a separate task.
> *Please help to triage the rest of TODOs.*
> A list of TODOs in our code:
> org/apache/reef/bridge/client/LocalClient.java
> // TODO: Make the parameters of the local runtime command line arguments of this tool.
> org/apache/reef/javabridge/FailedTaskBridge.java
> // TODO: deserialize/serialize with proper Avro schema
> org/apache/reef/javabridge/generic/Launch.java
> // TODO: Remove the injector, have stuff injected.
> org/apache/reef/driver/PreemptionEvent.java
> // TODO: We need to have a set of things to present to the user as preempted. Probably a Set<String> with the Evaluator IDs.
> org/apache/reef/runtime/common/driver/DriverStartHandler.java
> // TODO: We might have to indicate this to YARN somehow such that it doesn't try another time.
> (Driver restart happened, but no ON_DRIVER_RESTART handler is bound.)
> org/apache/reef/runtime/common/driver/evaluator/AllocatedEvaluatorImpl.java
> // TODO: The factories should be removed when deprecated setType is removed, as the process should not be created here
> org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
> // TODO: Eventually remove the factories when they are removed from AllocatedEvaluatorImpl
> org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
> // TODO: currently we obtain the job id directly by parsing execution (container) directory path
> // #845 is open to get the id from RM properly
> org/apache/reef/runtime/common/driver/evaluator/EvaluatorState.java
> // TODO: Add CLOSING state
> org/apache/reef/runtime/common/driver/resourcemanager/ResourceManagerStatus.java
> // TODO: Add state transition check
> org/apache/reef/runtime/common/evaluator/DefaultDriverConnection.java
> // TODO: implement a proper mechanism to obtain driver remote identifier.
> org/apache/reef/runtime/common/evaluator/HeartBeatManager.java
> // TODO: Write a test that checks for the order.
> org/apache/reef/runtime/common/evaluator/context/ContextRuntime.java
> // TODO: Which lock guards this?
> org/apache/reef/runtime/common/launch/LaunchClass.java
> // TODO: This probably should be bound via Tang.
> org/apache/reef/runtime/common/launch/REEFErrorHandler.java
> // TODO: This gets a new EventHandler each time an exception is caught. It would be better to cache the handler. But
> // that introduces threading issues and isn't really worth it, as the JVM typically will be killed once we catch an
> // Exception in here.
> org/apache/reef/runtime/common/launch/parameters/ErrorHandlerRID.java
> // TODO: Find all comparisons with this (NO_ERROR_HANDLER_REMOTE_ID) and see whether we can do something about them
> org/apache/reef/examples/suspend/Launch.java
> // TODO: Remove the injector, have stuff injected.
> org/apache/reef/examples/helloCLR/HelloCLR.java
> // TODO: Make this a config option
> org/apache/reef/io/data/loading/api/DataLoader.java
> // TODO: Add timeouts
> org/apache/reef/io/network/group/impl/operators/BroadcastReceiver.java
> // TODO: Should receive the identity element instead of null
> org/apache/reef/io/network/naming/NameLookupClient.java
> // TODO: better fix uses a map of id's after REEF-198
> org/apache/reef/io/network/naming/NameRegistryClient.java
> // TODO: better fix matches replies to threads with a map after REEF-198
> org/apache/reef/io/network/naming/NameServerImpl.java
> // TODO: All existing NameServer usage is currently new-up, need to make them injected as well.
> org/apache/reef/io/network/util/Utils.java
> // TODO: Merge with parseListCmp() into one generic implementation.
> org/apache/reef/io/storage/local/LocalScratchSpace.java
> // TODO: Error handling
> org/apache/reef/runtime/hdinsight/client/AzureUploader.java
> // TODO: If I don't do this check, the getLength() call below returns 0. No idea why.
> org/apache/reef/runtime/local/driver/ContainerManager.java
> // TODO: Find the actual system memory on this machine.
> org/apache/reef/runtime/mesos/client/MesosJobSubmissionHandler.java
> // The current implementation runs the driver as a local process, similar to reef-runtime-local.
> // TODO: run the driver on a slave node in the cluster
> org/apache/reef/runtime/mesos/driver/MesosResourceLaunchHandler.java
> // TODO: Replace REEFExecutor with a simple launch command (we only need to launch REEFExecutor)
> org/apache/reef/runtime/mesos/driver/REEFScheduler.java
> // Assumes for now that all slaves use port 5051(default) TODO: make it configurable.
> // TODO: make it (user?) configurable.
> // TODO: a hack to pass closeEvaluator test, replace this with a better interface
> // TODO: reflect priority and rack/node locality specified in resourceRequestEvent.
> org/apache/reef/runtime/mesos/evaluator/REEFExecutor.java
> // TODO: a hack to pass closeEvaluator test, replace this with a better interface
> org/apache/reef/runtime/mesos/util/MesosErrorHandler.java
> // ErrorHandler for MesosRemoteManager.
> // TODO: Replace this class once Tang's namespace feature is enabled
> org/apache/reef/runtime/mesos/util/MesosRemoteManager.java
> // Since the existing RemoteManager cannot use an additional codec,
> // we need this additional RemoteManager to use MesosMessageCodec.
> // TODO: Replace this class once Tang's namespace feature is enabled
> org/apache/reef/runtime/yarn/client/YarnJobSubmissionHandler.java
> // TODO: Revisit this. We also have a named parameter for the queue in YarnClientConfiguration.
> org/apache/reef/runtime/yarn/client/YarnSubmissionHelper.java
> // TODO: this is currently being developed on a hacked 2.4.0 bits, should be 2.4.1
> org/apache/reef/runtime/yarn/client/uploader/JobUploader.java
> // TODO: This really should be configurable, but wasn't in the code I moved as part of [REEF-228]
> org/apache/reef/runtime/yarn/driver/YarnContainerManager.java
> // TODO: return actual values for progress
> // TODO: this is currently being developed on a hacked 2.4.0 bits, should be 2.4.1
> // TODO: revisit this when implementing locality-strictness (i.e. a specific rack request can be ignored)
> // TODO: check vcores once YARN-2380 is resolved
> org/apache/reef/tests/fail/driver/FailDriver.java
> // TODO: notify client?
> org/apache/reef/tang/Configuration.java
> // TODO: Should this return Set<ConstructorDef<T>> instead?
> // TODO: This API seems wonky.  Why not return a Set<NamedParameterNode<Set<?>>> instead, and let the caller invoke getBoundSet() above?
> org/apache/reef/tang/formats/AvroConfigurationSerializer.java
> // TODO: This method should implement list deserialization. Implement it when C# side is ready.
> // TODO: This method should implement list serialization. Implement it when C# side is ready.
> org/apache/reef/tang/formats/CommandLine.java
> // TODO: Check for conflicting options.
> org/apache/reef/tang/formats/ConfigurationModuleBuilder.java
> // TODO: It would be nice if this incorporated d by reference so that static analysis / documentation tools
> // could document the dependency between c and d.
> org/apache/reef/tang/implementation/ConfigurationBuilderImpl.java
> // TODO: None of these should be public! - Move to configurationBuilder. Have
> // that wrap itself
> // in a sane Configuration interface...
> // TODO: Should be final again!
> org/apache/reef/tang/implementation/Constructor.java
> // TODO: use ArrayList internally (and maybe for input, too).
> org/apache/reef/tang/implementation/Subplan.java
> // TODO: use ArrayList internally (and maybe for input, too).
> org/apache/reef/tang/implementation/java/ClassHierarchyImpl.java
> // TODO: When handling sets, need to track target of generic parameter, and check the type here!
> org/apache/reef/tang/implementation/java/InjectorImpl.java
> // TODO: We should be insisting on injection futures here! .forkInjector();
> org/apache/reef/tang/implementation/java/InjectorImpl.java
> // TODO: Must be named parameter node.  Check.
> org/apache/reef/tang/implementation/java/JavaNodeFactory.java
> // TODO: When we use paramTypes below, we strip generic parameters.  Is that OK?
> org/apache/reef/tang/util/ReflectionUtilities.java
> // TODO: Float and double are currently coercible to int and long.  This is a bug.
> org/apache/reef/tang/util/Tint.java
> // TODO: Documentation string?
> org/apache/reef/wake/IdentifierParser.java
> // TODO: Modify tang to allow this to use a factory pattern.
> org/apache/reef/wake/impl/BlockingEventHandler.java
> // TODO: a queue is likely overly conservative given that we only need
> // to preserve order of those pairs of events that didn't race (have an ordering)
> // FIXME: There is a race here where the person draining the events might
> // not include their event as the last one. I'm going to assume this does not
> // matter, since all events will still be drained exactly once by someone in
> // the proper order
> // TODO: a non-locking implementation will simply atomically update the head of the 
> // queue to index=expectedSize, so that the drainer may drain without synchronization
> org/apache/reef/wake/impl/ForkPoolStage.java
> // TODO: should WakeSharedPool register its stages?
> org/apache/reef/wake/impl/WakeSharedPool.java
> // TODO: need to pass this upwards to REEF can grab it
> org/apache/reef/wake/rx/impl/TimeoutSubject.java
> // TODO: change Subject to specify conversion to T



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)