ctubbsii commented on a change in pull request #1907:
URL: https://github.com/apache/accumulo/pull/1907#discussion_r570301535
##########
File path: test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java
##########
@@ -162,7 +163,7 @@ public void checkHasMain() {
expectSet.add(Info.class);
expectSet.add(Initialize.class);
expectSet.add(LoginProperties.class);
- expectSet.add(Master.class);
+ expectSet.add(Manager.class);
Review comment:
Yes. That is why the test existed. When I originally added the
KeywordExecutable stuff, Eric had advocated for ensuring support for executing
server processes directly with their own main methods (via `java
path.to.actual.main.Class` or via `bin/accumulo path.to.actual.main.Class`),
because that had previously worked, and some users may still be executing them
that way.
However, this would *only* apply to the old class names that previously
existed with a main method, for backwards compatibility. Newer class names
would not already be in use by users, so they don't need to be introduced with
a main method, because there's no backwards compatibility argument to have one.
In the end, this argument for backwards compatibility is pretty obscure. It
is most likely that users are executing these with the scripts (which use our
`start.Main keyword` entry points). So, if we decide to remove the main
methods, that's fine... but it wouldn't make sense to rename the class in this
test, since the only point of this test is to check backwards compatibility of
the original class name having a main method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
