You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by bderak <gi...@git.apache.org> on 2015/09/10 17:19:19 UTC

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

GitHub user bderak opened a pull request:

    https://github.com/apache/flink/pull/1116

    [FLINK-2654][flink-java] Add JavaDoc to ParameterTool class

    Link to the Issue:
    https://issues.apache.org/jira/browse/FLINK-2654
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/bderak/flink add-java-docs-to-parameter-tool

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1116.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1116
    
----
commit 1a1c7477af5e028f04af55b8527cf82686bfd85a
Author: Behrouz Derakhshan <be...@gmail.com>
Date:   2015-09-10T14:02:51Z

    [FLINK-2654][flink-java] Add JavaDoc to ParameterTool class

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173169
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -124,6 +138,13 @@ else if(arg.startsWith("-")) {
     		return fromMap(map);
     	}
     
    +    /**
    --- End diff --
    
    The comments above are indented using tabs, here, you are switching to spaces.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1116#issuecomment-139528695
  
    Thanks for the update. Good to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173008
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -124,6 +138,13 @@ else if(arg.startsWith("-")) {
     		return fromMap(map);
     	}
     
    +    /**
    +     * Returns {@link ParameterTool} for the given properties file
    --- End diff --
    
    Maybe it would make sense to link to the documentation of Java Properties (so that people know how the format looks like)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1116


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173366
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -234,6 +295,10 @@ public long getLong(String key) {
     		return Long.valueOf(value);
     	}
     
    +	/**
    +	 * Returns the Long value for the given key. If the key does not exists it will return the default value given.
    +	 * The method fails if the value is not an Long.
    --- End diff --
    
    an long.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173419
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -245,13 +310,20 @@ public long getLong(String key, long defaultValue) {
     	}
     
     	// -------------- FLOAT
    -
    +	/**
    +	 * Returns the Float value for the given key.
    +	 * The method fails if the key does not exist.
    +	 */
     	public float getFloat(String key) {
     		addToDefaults(key, null);
     		String value = getRequired(key);
     		return Float.valueOf(value);
     	}
     
    +	/**
    +	 * Returns the Float value for the given key. If the key does not exists it will return the default value given.
    +	 * The method fails if the value is not an Float.
    --- End diff --
    
    an float.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/1116#issuecomment-139280931
  
    Thanks a lot for adding the Javadocs to the class. Good work!
    I have some comments in the diff.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39172874
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -124,6 +138,13 @@ else if(arg.startsWith("-")) {
     		return fromMap(map);
     	}
     
    +    /**
    +     * Returns {@link ParameterTool} for the given properties file
    +	 *
    --- End diff --
    
    the `*` is off here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by bderak <gi...@git.apache.org>.
Github user bderak commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173028
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -124,6 +138,13 @@ else if(arg.startsWith("-")) {
     		return fromMap(map);
     	}
     
    +    /**
    +     * Returns {@link ParameterTool} for the given properties file
    +	 *
    --- End diff --
    
    ouch, my IDEA defaults to 4 space indentation. 
    I ll fix it now..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1116#discussion_r39173067
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/utils/ParameterTool.java ---
    @@ -136,15 +157,35 @@ public static ParameterTool fromPropertiesFile(String path) throws IOException {
     		return fromMap((Map)props);
     	}
     
    +    /**
    +     * Returns {@link ParameterTool} for the given map
    +	 *
    +     * @param map A map of arguments. Both Key and Value have to be Strings
    +     * @return A {@link ParameterTool}
    +     */
     	public static ParameterTool fromMap(Map<String, String> map) {
     		Preconditions.checkNotNull(map, "Unable to initialize from empty map");
     		return new ParameterTool(map);
     	}
     
    +    /**
    +     * Returns {@link ParameterTool} from the system properties
    --- End diff --
    
    Maybe it would make sense to add that people can pass system properties to the jvm using `-Dkey=value`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1116#issuecomment-139559991
  
    will merge this


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2654][flink-java] Add JavaDoc to Parame...

Posted by bderak <gi...@git.apache.org>.
Github user bderak commented on the pull request:

    https://github.com/apache/flink/pull/1116#issuecomment-139288335
  
    The default indentation was switched to space rather than tab in my IntelliJ. It is fixed now.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---