Skip to content
This repository has been archived by the owner. It is now read-only.

rhbz1110627 zanata init #27

Closed
wants to merge 47 commits into from

Conversation

Projects
None yet
3 participants

@seanf seanf changed the title Rhbz1110627 zanata init rhbz1110627 zanata init Jul 17, 2014


import java.util.List;

public interface Interactiveable {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

The name Interactiveable is very awkward to interpret in English - I think the "able" suffix is only appropriate on verbs, not on adjectives. To mean "can be interacted with", Interactive would be closer in meaning, but since this is just for interacting with some kind of text console it could have a more specific name. It interacts with a user using strings that may have different display modes (i.e. styling), so maybe something like StyledTextInteractor.

* @author Patrick Huang <a
* href="mailto:pahuang@redhat.com">pahuang@redhat.com</a>
*/
public class InteractiveableFromConsole implements Interactiveable {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

This name has similar issue to the interface name with 'Interactiveable'. It could just be called ConsoleInteractor or something like that.

@Override
public void expectYes() {
String line = console.readLine();
Preconditions.checkNotNull(line, CONSOLE_CLOSE_ERROR);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Does console.readLine() return null when the console is closed before any input?

The check for non-null is duplicated 4 times here, and would be easy for someone to forget when adding another readLine(). I suggest extracting these two lines of code to a method like readNonNullLine() or readLineOrAbort().

}

@Override
public void expectYes() {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

This code is essentially identical to expectYes(Console) elsewhere. Will it be possible to unify them at some point?

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

Only publican-push and pull are using expectYes method from ConfigurableCommand. They are all deprecated class/commands. They are using system console whereas the new code is using ansi wrapped print stream. Since they are deprecated I don't want to mess with it.

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Thursday, July 17, 2014 3:11:00 PM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/InteractiveableFromConsole.java:

+/**

  • * @author Patrick Huang <a
  • * href="mailto:pahuang@redhat.com">pahuang@redhat.com
  • */
    +public class InteractiveableFromConsole implements Interactiveable {
  • private static final String CONSOLE_CLOSE_ERROR = "console stream
    closed";
  • private final Console console;
  • private final PrintStream out;
  • public InteractiveableFromConsole() {
  •    console = System.console();
    
  •    out = AnsiConsole.out();
    
  • }

This code is essentially identical to expectYes(Console) elsewhere. Will it
be possible to unify them at some point?


Reply to this email directly or view it on GitHub .

* If user answer something that is out of the expected answers list, it
* will give a warning and offer retry
*/
String expectAnswerWithRetry(List<String> expectedAnswers);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

It is surprising that this takes List<String> whereas expectAnswer() takes String.... Their names and descriptions are almost identical.

This comment has been minimized.

Copy link
@huangp

huangp Jul 18, 2014

Author Collaborator

I use an interface just so I can replace it with something else in test. The methods do not carry any API information is just it's easier for me to call. In some cases I call it with varargs and in other cases I call it with list. But the implementations are different (one will terminate if you answer something invalid and one will offer retry. But after thinking again I think offering retry in all cases should be more appropriate. I will rename the method name to reflect that.


Interactiveable printfln(Mode mode, String printfFmt, Object... args);

String expectAnyNotBlankAnswer();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Does "blank" mean answer.length != 0, or answer.trim().length != 0? I would at least like a description to indicate what will be considered blank, but an alternative name may be a more concise way to communicate the behaviour, depending what it is (like expectAnyNonWhitespaceAnswer() or whatever).

This comment has been minimized.

Copy link
@huangp

huangp Jul 18, 2014

Author Collaborator

Commons string util has isBlank method. It interpret any white space, tab, newline maybe a few others as well etc as blank. I am just following that convention to call it NotBlank.


String expectAnyNotBlankAnswer();

String expectAnyAnswer();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Since there are several methods that expect different types of answers, it might be more flexible and easier to extend if there was just one method that takes a ResponseValidator, and provide some default validators as static fields. e.g. String answer = expectAnswer(ANY), String answer = expectAnswer(NOT_BLANK) or String answer = expectAnswer(anyOf(someStrings)). Any of these could also work with expectAnswerWithRetry().

This comment has been minimized.

Copy link
@huangp

huangp Jul 18, 2014

Author Collaborator

I gave it a go but realize the benefit is minimal. For each case the invalid path is different (different error message or handling etc). I then took the approach of guava utility class which offer a rich, some what duplicated method set just so you can call it easily. (like ImmutableList will take one, two, varargs, list every possible way to create an instance)


String expectAnyAnswer();

enum Mode {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Mode is just for display, right? It could be DisplayMode or PrintMode to make this explicit.

This comment has been minimized.

Copy link
@huangp

huangp Jul 18, 2014

Author Collaborator

Yep. So we won't depend on Jansi's API directly and give us the chance to change to something else without breaking the client. This is more to describe the message purpose. A bit like log level. I think DisplayMode sound better.

public String expectAnswerWithRetry(List<String> expectedAnswers) {
String line = console.readLine();
Preconditions.checkNotNull(line, CONSOLE_CLOSE_ERROR);
if (!expectedAnswers.contains(line)) {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

I think this would be more readable if inverted.

if (bundle.containsKey(key)) {
return bundle.getString(key);
}
log.warn("Cannot find key [{}] in resource bundle", key);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Is there a situation in which we would want to ship a program that is missing any of the prompt strings? If not, we may want to have this terminate the program instead so that we can't accidentally ship a broken program.

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Actually it would still compile so we could still miss something - it should log as an error at least because the client is broken if a display string is missing.

This comment has been minimized.

Copy link
@huangp

huangp Jul 18, 2014

Author Collaborator

I will change it to error level

return !(value instanceof Collection) || !((Collection) value).isEmpty();
}

private boolean ifStringNotBlank(T value) {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

These do not conditionally execute code, so I think they should just be called notEmptyCollection and notBlankString. I think these names also give more of a hint that it is checking the type as well - the current names seem to suggest that you could only give a Collection or a String as the argument.

* @param configExample
* how it should be defined in zanata.xml
*/
public void hintUserToUseConfig(String configExample) {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

This does not always show the hint, so the name should indicate that it is conditional, e.g. logHintIfNotInConfig(...).

* If option values mismatch between project config and given value, log a
* warning.
*/
public void warnUserForMismatch() {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Similar to above, this could be logWarningIfValuesMismatch(...) so that it is clear at the call site that it is conditional. With the current name, it looks like there is a mistake in the control logic, even though there is not.

}
transDirChecker.hintUserToUseConfig(String.format(
"<trans-dir>%s</trans-dir>", opts.getTransDir()));
transDirChecker.warnUserForMismatch();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Identical control flow is repeated twice here. Can it be abstracted out?

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

Actually it looks like it is repeated below as well.

Could we just have a superclass that does the control flow, and a subclass for each expected option that overrides functions for extracting their values from config/options and generating hints?

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

It's not identical. The expression inside the if block and the hint message are different. Unless we use reflection (i.e. to replace that setter and getter call) I can't think of any way to extract that commonality.
----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Thursday, July 17, 2014 5:49:58 PM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/OptionsUtil.java:

  •        opts.setSrcDir(config.getSrcDirAsFile());
    
  •    }
    
  •    srcDirChecker.hintUserToUseConfig(
    
  •            String.format("<src-dir>%s</src-dir>", opts.getSrcDir()));
    
  •    srcDirChecker.warnUserForMismatch();
    
  •    // apply transDir configuration
    
  •    OptionMismatchChecker<File> transDirChecker =
    
    OptionMismatchChecker
  •            .from(opts.getTransDir(), config.getTransDirAsFile(),
    
  •                    "Translation directory");
    
  •    if (transDirChecker.hasValueInConfigOnly()) {
    
  •        opts.setTransDir(config.getTransDirAsFile());
    
  •    }
    
  •    transDirChecker.hintUserToUseConfig(String.format(
    
  •            "<trans-dir>%s</trans-dir>", opts.getTransDir()));
    
  •    transDirChecker.warnUserForMismatch();
    

Identical control flow is repeated twice here. Can it be abstracted out?


Reply to this email directly or view it on GitHub .

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

Almost no gain but one extra layer of abstraction. Again unless we ditch strongly typed setter/getter and use reflection (or map), I don't think we can avoid this.

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Thursday, July 17, 2014 5:56:02 PM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/OptionsUtil.java:

  •        opts.setSrcDir(config.getSrcDirAsFile());
    
  •    }
    
  •    srcDirChecker.hintUserToUseConfig(
    
  •            String.format("<src-dir>%s</src-dir>", opts.getSrcDir()));
    
  •    srcDirChecker.warnUserForMismatch();
    
  •    // apply transDir configuration
    
  •    OptionMismatchChecker<File> transDirChecker =
    
    OptionMismatchChecker
  •            .from(opts.getTransDir(), config.getTransDirAsFile(),
    
  •                    "Translation directory");
    
  •    if (transDirChecker.hasValueInConfigOnly()) {
    
  •        opts.setTransDir(config.getTransDirAsFile());
    
  •    }
    
  •    transDirChecker.hintUserToUseConfig(String.format(
    
  •            "<trans-dir>%s</trans-dir>", opts.getTransDir()));
    
  •    transDirChecker.warnUserForMismatch();
    

Actually it looks like it is repeated below as well.

Could we just have a superclass that does the control flow, and a subclass
for each expected option that overrides functions for extracting their
values from config/options and generating hints?


Reply to this email directly or view it on GitHub .


/**
* Note: command line options take precedence over pom.xml which
* takes precedence over zanata.xml.

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

The ideal approach would be to express this in code. We really should not have to go through each parameter individually and repeatedly implement this relationship.

return true;
}
// read the content and see if we need to check
Properties props = loadFileToProperties(updateMarker);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

What happens if a user edits this file and makes it invalid? Will they get a sensible error message, or will their client spit out a stack trace? We do expect people to edit the file, so it is important to make sure it behaves in a way that will support that.

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

Since we won't be including the update checker in (pending proper story and RFE), I will just comment out the usage of this class but leave it there for reference.

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Friday, July 18, 2014 9:56:38 AM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/UpdateChecker.java:

  •    this.console = console;
    
  •    this.currentVersionNo = currentVersionNo;
    
  •    this.updateMarker = updateMarker;
    
  • }
  • public boolean needToCheckUpdates(boolean interactiveMode) {
  •    DateTime today = new DateTime();
    
  •    try {
    
  •        if (!updateMarker.exists()) {
    
  •            createUpdateMarkerFile(updateMarker);
    
  •            console.printfln(_("update.marker.created"),
    
    updateMarker);
  •            console.printfln(_("update.marker.hint"));
    
  •            return true;
    
  •        }
    
  •        // read the content and see if we need to check
    
  •        Properties props = loadFileToProperties(updateMarker);
    

What happens if a user edits this file and makes it invalid? Will they get a
sensible error message, or will their client spit out a stack trace? We do
expect people to edit the file, so it is important to make sure it behaves
in a way that will support that.


Reply to this email directly or view it on GitHub .

DateTime today = new DateTime();
try {
if (!updateMarker.exists()) {
createUpdateMarkerFile(updateMarker);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 17, 2014

Contributor

What will happen if there is a problem creating the file? Will the user see a warning message and still be able to use the client?

*
* @return latest version of client in sonatype oss
*/
private Optional<String> checkLatestVersion(Interactiveable console) {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

I think this method has too much responsibility - it should not deal with the console, the caller can print the failure message if this returns Optional.absent(). Without the console, this can be reused in other contexts.

Pattern pattern = Pattern.compile("^.+<version>(.+)</version>.+");
Matcher matcher = pattern.matcher(payload);
return matcher.matches() ? Optional.of(matcher.group(1)) : Optional
.<String> absent();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

This could go in a separate method rather than commenting that it is parsing xml cheaply.

Days days() {
switch (this) {
case monthly:
return Days.days(30);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

This will only be accurate a third of the time. We might want to call it everyThirtyDays and present it to the user that way. On the other hand, I doubt anyone would notice the inaccuracy in practice.

case daily:
return Days.ONE;
default:
return Weeks.ONE.toStandardDays();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

This is an exhaustive pattern, so I think it would be clearer to use case weekly: rather than default:.

public InitCommand(InitOptions opts) {
// we don't have all mandatory information yet. Can't create a real
// proxy factory.
super(opts, mockFactory);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

It seems like there must be something wrong with our inheritance hierarchy if we have to do this. Are there any plans to deal with that at some point?

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

This is the only class in the hierarchy that does not have mandatory information. I won't say the inheritance model is wrong. We just don't have a case for this situation in the past. As for now it should be fine since this is the only case. If it appears again we will think about it.

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Friday, July 18, 2014 10:37:56 AM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/init/InitCommand.java:

  • * @author Patrick Huang <a
  • * href="mailto:pahuang@redhat.com">pahuang@redhat.com
  • */
    +public class InitCommand extends ConfigurableCommand {
  • private static final Logger log = LoggerFactory
  •        .getLogger(InitCommand.class);
    
  • private static final ZanataProxyFactory mockFactory =
  •        MockZanataProxyFactory.mockFactory;
    
  • private static final String ITERATION_URL = "%siteration/view/%s/%s";
  • private Interactiveable interactiveable;
  • private ZanataProxyFactory requestFactory;
  • public InitCommand(InitOptions opts) {
  •    // we don't have all mandatory information yet. Can't create a
    
    real
  •    // proxy factory.
    
  •    super(opts, mockFactory);
    

It seems like there must be something wrong with our inheritance hierarchy if
we have to do this. Are there any plans to deal with that at some point?


Reply to this email directly or view it on GitHub .

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 28, 2014

Contributor

It may not have been wrong before, but since we now have a case that does not fit it, it must no longer be the best fit for our commands. If we expand the hierarchy further we should update it to avoid the mocking.

}

@Override
protected void run() throws Exception {

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

This method is responsible for the overall control flow of of the command. It would be more readable if that was its only responsibility - there are a number of places where it deals with lower level stuff.

Basically where there are comments like // Search for zanata.ini, // If there's zanata.xml, ask the user and // Select or create a project and version, they are flagging a block that could be moved to a function, so this would end up looking like:

    searchForZanataIni();
    createZanataXmlAndWarnUserIfOverwriting();
    selectOrCreateProjectAndVersion();
    getSrcAndTransDirFromUser();
    displayAdviceAboutUploadingStrings();

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 29, 2014

Contributor

(copied from comment on commit)
This has been improved, but still is difficult to look at and get an overview of what it is doing. This method's responsibility is obviously flow control, but it is taking on other responsibilities. Consider someone looking at this a year from now who just wants to make a change something about the translation directory prompt - as it is now they would have to wade through a whole lot of low-level detail mixed in with this overall control flow.

Level preLevel = logger.getLevel();
logger.setLevel(Level.OFF);
OptionsUtil.applyConfigFiles(getOpts());
logger.setLevel(preLevel);

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

This block might be worth moving to a new function in OptionsUtil such as applyConfigFilesQuietly(...) rather than mix low-level logging logic in here.

}

/**
* Downloads the zanata.xml config file (new REST endpoint).

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

A year from now, (new REST endpoint) will be inaccurate. Better to just say "Downloads the zanata.xml config file from a REST endpoint."

import org.zanata.client.commands.ConfigurableProjectOptions;

public interface InitOptions extends ConfigurableProjectOptions {
}

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

Would we lose something if we just used the ConfigurableProjectOptions interface for the options implementation to init command?

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

We won't lose anything for now. But if we ever have specific options in init, we will need this class. It will make our life easier having this class now. It also make the code easier to read.

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Friday, July 18, 2014 12:04:39 PM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/init/InitOptions.java:

@@ -0,0 +1,6 @@
+package org.zanata.client.commands.init;
+
+import org.zanata.client.commands.ConfigurableProjectOptions;
+
+public interface InitOptions extends ConfigurableProjectOptions {
+}

Would we lose something if we just used the ConfigurableProjectOptions
interface for the options implementation to init command?


Reply to this email directly or view it on GitHub .

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 28, 2014

Contributor

Code readability sounds like a sufficient reason here, and keeping things consistent, so that sounds fine.

interactiveable
.printfln(Confirmation, _("backup.old.project.config"), backup);
// ConfigurableMojo will set some options using existing config.
// Need to clear it.

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

Everything below here could be moved to a function like clearDefaultsFromConfigurableMojo().

@davidmason

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2014

@huangp a few of these files seem to be missing copyright notices. Can you check through them and add any that are missing?

*/
public void selectOrCreateNewProjectAndVersion() {
interactiveable.printf(Question, _("project.select.or.create"));
String answer = interactiveable.expectAnswer("1", "2");

This comment has been minimized.

Copy link
@davidmason

davidmason Jul 18, 2014

Contributor

Will this terminate the command if someone accidentally types something else?

This comment has been minimized.

Copy link
@huangp

huangp Jul 21, 2014

Author Collaborator

Changed to let them retry

----- Original Message -----

From: "David Mason" notifications@github.com
To: "zanata/zanata-client" zanata-client@noreply.github.com
Cc: "Patrick Huang" pahuang@redhat.com
Sent: Friday, July 18, 2014 12:59:08 PM
Subject: Re: [zanata-client] rhbz1110627 zanata init (#27)

In
zanata-client-commands/src/main/java/org/zanata/client/commands/init/ProjectPrompt.java:

  • ProjectPrompt(Interactiveable interactiveable, InitOptions opts,
  •        ZanataProxyFactory proxyFactory,
    
  •        ProjectIterationPrompt projectIterationPrompt) {
    
  •    this.interactiveable = interactiveable;
    
  •    this.opts = opts;
    
  •    this.proxyFactory = proxyFactory;
    
  •    this.projectIterationPrompt = projectIterationPrompt;
    
  • }
  • /**
  • \* Select or create a project and version.
    
  • \* If creating, also ask for Project type.
    
  • */
    
  • public void selectOrCreateNewProjectAndVersion() {
  •    interactiveable.printf(Question, _("project.select.or.create"));
    
  •    String answer = interactiveable.expectAnswer("1", "2");
    

Will this terminate the command if someone accidentally types something else?


Reply to this email directly or view it on GitHub .

@huangp huangp force-pushed the rhbz1110627-zanata-init branch 3 times, most recently from 9d4a74a to 55c1bf6 Aug 21, 2014


private static int firstDiffOrdinalOrLastOrdinalInShortestVer(
List<String> ver1Ordinals,
List<String> ver2Ordinals) {

This comment has been minimized.

Copy link
@davidmason

davidmason Aug 21, 2014

Contributor

I suggest using left and right from here down as well to be consistent.

@huangp huangp force-pushed the rhbz1110627-zanata-init branch 2 times, most recently from eb75f43 to f5f215c Aug 21, 2014


private static int compareSnapshot(String left, String right) {
boolean ver1IsSnapshot = left.endsWith(SNAPSHOT_SUFFIX);
boolean ver2IsSnapshot = right.endsWith(SNAPSHOT_SUFFIX);

This comment has been minimized.

Copy link
@davidmason

davidmason Aug 21, 2014

Contributor

Still more variables using ver1... and ver2..., I suggest a text search in the file to ensure all are updated.

@huangp huangp force-pushed the rhbz1110627-zanata-init branch from f5f215c to 7ab5819 Aug 21, 2014

@davidmason

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2014

👍

@huangp huangp added ON_QA and removed In progress labels Aug 21, 2014

@definite

This comment has been minimized.

Copy link
Member

commented Aug 21, 2014

Consider the scope of this bug, here is the new check list:

On unsupported server version handling:

  • The error message should be simpler, like "Unsupported server version".
  • It should be shown right after you choose server, not wait until user type a lot about project
  • Don't ask "Do you want to try again (y/n)

On source dir handling:

  • Instead of quitting after enter absent source dir, ask agin.
@definite

This comment has been minimized.

Copy link
Member

commented Aug 27, 2014

Currently, I need to edit org/zanata/client/commands/init/InitCommand.java: line 143, change it to 3.4.0-SNAPSHOT.

The PR will be verified again when final version is ready.

@definite

This comment has been minimized.

Copy link
Member

commented Aug 27, 2014

https://bugzilla.redhat.com/show_bug.cgi?id=1134146 is the RFE for keeping existing values for src-dir and trans-dir.

@definite definite added Verified and removed On QA Reviewed labels Sep 24, 2014

huangp added a commit that referenced this pull request Sep 24, 2014

rhbz1110627 rhbz1038852 - Squashed commit for zanata init command
#27

commit 7ab5819
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Aug 21 12:14:46 2014 +1000

    rhbz1110627 - handle not exist src dir

commit 8dea609
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Aug 21 11:55:58 2014 +1000

    rhbz1110627 - check server is compatible to init command

commit 61fe130
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 14:57:43 2014 +1000

    messages change

commit b493a38
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 12:25:40 2014 +1000

    change message template to match prompt

commit 93173dd
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 10:25:16 2014 +1000

    fix exception type and tests

commit cc4b7d2
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 14:03:11 2014 +1000

    change copyright year and a few messages

commit 94ec9ad
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:23:29 2014 +1000

    improve message

commit f383bd8
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:22:04 2014 +1000

    remove unique from project id prompt

commit b833d76
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:15:48 2014 +1000

    rhbz1110627 - handle when there is no server url in user config

commit cec96f2
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 11:51:39 2014 +1000

    refactoring

commit 3c4f815
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 09:15:46 2014 +1000

    rhbz1110627 - rename method and static import

commit f9e57f8
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 22 08:48:25 2014 +1000

    rhbz1110627 - refactoring

commit 5909e15
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 16:03:24 2014 +1000

    rhbz1110627 - extract methods in UserConfigHandler

commit 04286ba
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:58:43 2014 +1000

    rhbz1110627 - refactor project, source and trans prompt

commit cdf0efc
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:15:39 2014 +1000

    add copyright to files

commit d65ed05
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:10:47 2014 +1000

    rhbz1110627 - extract methods in ProjectConfigHandler

commit 65a437d
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:08:35 2014 +1000

    rhbz1110627 - extract methods in InitCommand

commit c6dbf0b
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:40:38 2014 +1000

    remove UpdateChecker from production code (pending story and RFE)

commit 0fa26af
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:32:49 2014 +1000

    rhbz1110627 - missing resource key will log an error

commit 471e586
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:23:40 2014 +1000

    rhbz1110627 - rename methods in OptionMismatchChecker

commit 19c50eb
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:15:25 2014 +1000

    rhbz1110627 - rename class Mode to DisplayMode

commit ac0cb55
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:09:07 2014 +1000

    rhbz1110627 - rename class

commit 8804142
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:07:27 2014 +1000

    rhbz1110627 - refactor InitCommand: extract method etc

commit 51a6f99
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:00:19 2014 +1000

    rhbz1110627 - refactor class that expects user input

commit 9db3a26
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jul 17 11:55:58 2014 +1000

    rhbz1110627 - add color output and change some messages

commit 85f9c01
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jul 16 09:38:05 2014 +1000

    rhbz1110627 - able to filter project in long list

commit a8fa923
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jul 11 13:09:08 2014 +1000

    rhbz1110627 - check key availability in resource bundle

commit 99a9304
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jul 9 15:51:01 2014 +1000

    rhbz1110627 - check if there is newer version of client available

commit 5fe89e2
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jul 3 11:11:12 2014 +1000

    rhbz1110627 - externalize strings to properties file

commit e57037a
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 13:36:48 2014 +1000

    rhbz1110627 - add init to ZanataClient

commit 8c0c01d
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 13:36:23 2014 +1000

    rhbz1110627 - fix minor issues and print what's next suggestion to user

commit 60e557a
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 11:28:57 2014 +1000

    rhbz1110627 - refactor and clean up code

commit 549f095
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 10:53:38 2014 +1000

    rhbz1110627 - refactor to extract project and version prompt to separate classes

commit 769caf5
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 09:32:06 2014 +1000

    rhbz1110627 - refactor extract project config handling to separate class

commit d001f2b
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 30 16:59:41 2014 +1000

    rhbz1110627 - refactor extract to separate class

commit 1f11d50
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 30 16:37:16 2014 +1000

    rhbz1110627 - refactor test and mock server

commit 11c2d46
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 15:59:06 2014 +1000

    rhbz1110627 - add init mojo as maven goal

commit 531f2cc
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 15:57:29 2014 +1000

    rhbz1110627 - prompt user for trans dir

commit 96a8623
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 14:40:27 2014 +1000

    rhbz1110627 - use index number to select project and version

commit 49cb2b6
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 14:08:59 2014 +1000

    rhbz1110627 - src dir, includes and excludes prompt

commit 9061f89
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 12:20:09 2014 +1000

    rhbz1110627 - refactor ZanataProxyFactory to allow not eager REST version call

commit 508ec9d
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 14:17:18 2014 +1000

    rhbz1110627 - add includes and excludes to zanata config

commit c48414a
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 10:59:36 2014 +1000

    rhbz1110627 - pull up includes and excludes to project configuration

commit 2d1a9b3
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 10:26:46 2014 +1000

    rhbz1110627,rhbz1038852 - add src dir and trans dir to zanata config

commit 31319e5
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 09:13:45 2014 +1000

    check for potential wrong src dir in podir project

commit 8e05950
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 23 09:26:25 2014 +1000

    rhbz1110627 - create init command

commit c27ebac
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jun 25 10:17:02 2014 +1000

    add friendlier message to inform user server is not up
@huangp

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 24, 2014

squash merged

@huangp huangp closed this Sep 24, 2014

@huangp huangp deleted the rhbz1110627-zanata-init branch Sep 24, 2014

seanf pushed a commit to zanata/zanata-platform that referenced this pull request Oct 20, 2016

rhbz1110627 rhbz1038852 - Squashed commit for zanata init command
zanata/zanata-client#27

commit 7ab5819cefde4b76a6256cc5ecd5e15435b5df73
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Aug 21 12:14:46 2014 +1000

    rhbz1110627 - handle not exist src dir

commit 8dea609e2c52d70bd6c7db501a5d946e3eea7215
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Aug 21 11:55:58 2014 +1000

    rhbz1110627 - check server is compatible to init command

commit 61fe130f878e7c29798264c9f8188726730700f4
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 14:57:43 2014 +1000

    messages change

commit b493a38bd6c1fc71b4a2a2d61b2c4d586717a96e
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 12:25:40 2014 +1000

    change message template to match prompt

commit 93173dd4bc829334827a29c759754355889a4584
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Aug 5 10:25:16 2014 +1000

    fix exception type and tests

commit cc4b7d2c62042752e3fc20b58964c28c2ff43e34
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 14:03:11 2014 +1000

    change copyright year and a few messages

commit 94ec9adde1e5dbec3d8aa777f4c3736992493d31
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:23:29 2014 +1000

    improve message

commit f383bd89ab1f3d2e36de8169fee851cbd0533263
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:22:04 2014 +1000

    remove unique from project id prompt

commit b833d7608cbddc6dbc8e82f5d6a9a07cbf280f79
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 12:15:48 2014 +1000

    rhbz1110627 - handle when there is no server url in user config

commit cec96f250a5a8ff0ba9588986bb2a65a1d137f9e
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 11:51:39 2014 +1000

    refactoring

commit 3c4f815f66d48fa539df07606091435a0939d2d5
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 29 09:15:46 2014 +1000

    rhbz1110627 - rename method and static import

commit f9e57f8d5fb1de5dd0bdc29f32a5ebc14b64d32c
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 22 08:48:25 2014 +1000

    rhbz1110627 - refactoring

commit 5909e153ad6fa058e672a994b7b756990b62a628
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 16:03:24 2014 +1000

    rhbz1110627 - extract methods in UserConfigHandler

commit 04286baa5eab756dc9d025ebbf391046537db253
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:58:43 2014 +1000

    rhbz1110627 - refactor project, source and trans prompt

commit cdf0efc53baeac5192484c7d45548161d539785d
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:15:39 2014 +1000

    add copyright to files

commit d65ed059d8558ab08bca3baa4da21c084110bf98
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:10:47 2014 +1000

    rhbz1110627 - extract methods in ProjectConfigHandler

commit 65a437ddda8afaee24f3880410e53a0982ea7055
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 15:08:35 2014 +1000

    rhbz1110627 - extract methods in InitCommand

commit c6dbf0b40ef1471ab3b8980d17c8c471561954cc
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:40:38 2014 +1000

    remove UpdateChecker from production code (pending story and RFE)

commit 0fa26af4e5a4068ad5bd5ccbf9c3b92a4c85f4b0
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:32:49 2014 +1000

    rhbz1110627 - missing resource key will log an error

commit 471e5863a49253235892f2f118f4614f7962f0ec
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:23:40 2014 +1000

    rhbz1110627 - rename methods in OptionMismatchChecker

commit 19c50ebebd8bc60935deb72469b99268ee973634
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:15:25 2014 +1000

    rhbz1110627 - rename class Mode to DisplayMode

commit ac0cb550bff27c6c647091b8be2815f1299e8c06
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:09:07 2014 +1000

    rhbz1110627 - rename class

commit 8804142bb0c9474bfc4f811da169df2bae7cb204
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:07:27 2014 +1000

    rhbz1110627 - refactor InitCommand: extract method etc

commit 51a6f99f27e55e207cb3f43e0602eb1e9a0553ef
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jul 21 14:00:19 2014 +1000

    rhbz1110627 - refactor class that expects user input

commit 9db3a2696f6dcb2e0de58b2276e804d91ade5a09
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jul 17 11:55:58 2014 +1000

    rhbz1110627 - add color output and change some messages

commit 85f9c01a297683251383f7725fc63c831b19ee85
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jul 16 09:38:05 2014 +1000

    rhbz1110627 - able to filter project in long list

commit a8fa92300fee028019a20e2b45acd76aa3d3559f
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jul 11 13:09:08 2014 +1000

    rhbz1110627 - check key availability in resource bundle

commit 99a93043201a3ffe34b2596292293de537861576
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jul 9 15:51:01 2014 +1000

    rhbz1110627 - check if there is newer version of client available

commit 5fe89e2a7e90cb6fc15e7fa2906cf53b15c9daf6
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jul 3 11:11:12 2014 +1000

    rhbz1110627 - externalize strings to properties file

commit e57037a32939e0b2629ebfebc56e38b0a9eb289d
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 13:36:48 2014 +1000

    rhbz1110627 - add init to ZanataClient

commit 8c0c01d3b7c8033651b8fa119b04c3add09dcc4f
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 13:36:23 2014 +1000

    rhbz1110627 - fix minor issues and print what's next suggestion to user

commit 60e557a46f9f6682b3054f32cba601f5137f6618
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 11:28:57 2014 +1000

    rhbz1110627 - refactor and clean up code

commit 549f095f51d81686845c8ced7692b96600d6769a
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 10:53:38 2014 +1000

    rhbz1110627 - refactor to extract project and version prompt to separate classes

commit 769caf5c987988f8b13a3423e0ff7d19bdf9ce14
Author: Patrick Huang <pahuang@redhat.com>
Date:   Tue Jul 1 09:32:06 2014 +1000

    rhbz1110627 - refactor extract project config handling to separate class

commit d001f2be0a906b1f9eb3ed4c228f42fede14edc7
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 30 16:59:41 2014 +1000

    rhbz1110627 - refactor extract to separate class

commit 1f11d50717658730f027d1733dc0404e1619cca2
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 30 16:37:16 2014 +1000

    rhbz1110627 - refactor test and mock server

commit 11c2d46b65a46d0ec297767e5fc5213644031bdc
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 15:59:06 2014 +1000

    rhbz1110627 - add init mojo as maven goal

commit 531f2cc2769b27ef74f7ba244c8ded383c3359c0
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 15:57:29 2014 +1000

    rhbz1110627 - prompt user for trans dir

commit 96a86230847a230221a027a54500c9da66d7c41e
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 14:40:27 2014 +1000

    rhbz1110627 - use index number to select project and version

commit 49cb2b632dfc8633baf7d514d275e5b6ae6669dc
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 14:08:59 2014 +1000

    rhbz1110627 - src dir, includes and excludes prompt

commit 9061f89487e2a52cb3e400f2ff7a2982ac3c3fd7
Author: Patrick Huang <pahuang@redhat.com>
Date:   Fri Jun 27 12:20:09 2014 +1000

    rhbz1110627 - refactor ZanataProxyFactory to allow not eager REST version call

commit 508ec9dd508f4e1768682806ddbc85361f24c588
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 14:17:18 2014 +1000

    rhbz1110627 - add includes and excludes to zanata config

commit c48414a13da6cc787402d28a2ec3c3727b513691
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 10:59:36 2014 +1000

    rhbz1110627 - pull up includes and excludes to project configuration

commit 2d1a9b34bd110b5b61b1ca419a1535510a60a194
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 10:26:46 2014 +1000

    rhbz1110627,rhbz1038852 - add src dir and trans dir to zanata config

commit 31319e5ef9629e8b6393658a093c4a4f21661ac8
Author: Patrick Huang <pahuang@redhat.com>
Date:   Thu Jun 26 09:13:45 2014 +1000

    check for potential wrong src dir in podir project

commit 8e0595017a6d362a9edeb3f8a0d86be6d03cff84
Author: Patrick Huang <pahuang@redhat.com>
Date:   Mon Jun 23 09:26:25 2014 +1000

    rhbz1110627 - create init command

commit c27ebac5da785fd3fc2f31c81212bbecdc9e9709
Author: Patrick Huang <pahuang@redhat.com>
Date:   Wed Jun 25 10:17:02 2014 +1000

    add friendlier message to inform user server is not up
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.