[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
GitHub user arcadius opened a pull request:

    https://github.com/apache/ant/pull/31

    Feature/bugzilla bug 60628 ant get task to accept arbitrary header

    [AA] bugzilla Bug 60628 Ant-Get-Task-To-Accept-Arbitrary-Header


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

    $ git pull https://github.com/MenelicSoftware/ant feature/bugzilla-Bug-60628-Ant-Get-Task-To-Accept-Arbitrary-Header

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

    https://github.com/apache/ant/pull/31.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 #31
   
----
commit b83bdcebf0395250b3ae5e7e4af94ff1400493f0
Author: Arcadius Ahouansou <[hidden email]>
Date:   2017-01-23T22:46:06Z

    [AA] Bugzilla Bug 60628 - Ant Get Task To Accept Arbitrary Header

commit 3d215101449f63be2c62baf49425a929d758168f
Author: Arcadius <[hidden email]>
Date:   2017-01-23T23:50:55Z

    [AA]  bugzilla-Bug-60628-Ant-Get-Task-To-Accept-Arbitrary-Header : simplifying test

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
Github user arcadius commented on the issue:

    https://github.com/apache/ant/pull/31
 
    @mbenson  , would you be able to review this pull request for me?
    It is very similar to the email header that you worked on.
   



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
In reply to this post by apupier
Github user janmaterne commented on the issue:

    https://github.com/apache/ant/pull/31
 
    Haven't done a deep check, but first comments:
    - overall it seems to be ok
    - get.html: maybe a hint that key and value are trimmed and must not be null or empty string
    - get.html, example: value has doubled equals sign
    - get.xml/GetTest.java: method name doesn't match target name
    - Get.java: nice to reuse email.Header
    - Get.java: trimToNull() could be moved to org.apache.tools.ant.util.StringUtils
    - Get.java:764: the if(isEmpty) is not required
   
    @mbenson: Are you reviewing 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user arcadius commented on a diff in the pull request:

    https://github.com/apache/ant/pull/31#discussion_r98552982
 
    --- Diff: manual/Tasks/get.html ---
    @@ -170,6 +170,29 @@
       name will be skipped.  If the returned name is a relative path, it
       will be considered relative to the <em>dest</em> attribute.</p>
     
    +<h4>header</h4>
    +<p>Any arbitrary number of HTTP headers can be added to a request.<br/>
    +  The attributes of a nested <pre>&lt;header/&gt; </pre> node are as follow:
    +<p></p>
    +
    +<table width="60%" border="1" cellpadding="2" cellspacing="0">
    +  <tr>
    +    <td valign="top"><b>Attribute</b></td>
    +    <td valign="top"><b>Description</b></td>
    +    <td align="center" valign="top"><b>Required</b></td>
    +  </tr>
    +  <tr>
    +    <td valign="top">name</td>
    +    <td valign="top">The name or key of this header.</td>
    --- End diff --
   
    Added comment about empty space being trimmed


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user arcadius commented on a diff in the pull request:

    https://github.com/apache/ant/pull/31#discussion_r98553907
 
    --- Diff: manual/Tasks/get.html ---
    @@ -234,6 +257,17 @@
       &lt;url url=&quot;http://ant.apache.org/faq.html&quot;/&gt;
     &lt;/get&gt;
     </pre>
    +
    +<p>With custom HTTP headers</p>
    +<pre>
    +&lt;get src=&quot;http://ant.apache.org/index.html&quot; dest=&quot;downloads&quot;&gt;
    +  &lt;header name=&quot;header1&quot; value==&quot;headerValue1&quot; /&gt;
    --- End diff --
   
    fixed typo ```==```


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user arcadius commented on a diff in the pull request:

    https://github.com/apache/ant/pull/31#discussion_r98555008
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/Get.java ---
    @@ -726,6 +761,14 @@ private URLConnection openConnection(final URL aSource) throws IOException {
                     connection.setRequestProperty("Accept-Encoding", GZIP_CONTENT_ENCODING);
                 }
     
    +            if (!headers.isEmpty()) {
    --- End diff --
   
    Yes, agree, ```headers``` empty check is not necessary +1


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user arcadius commented on a diff in the pull request:

    https://github.com/apache/ant/pull/31#discussion_r98557699
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/Get.java ---
    @@ -483,6 +489,35 @@ public void setTryGzipEncoding(boolean b) {
         }
     
         /**
    +     * Add a nested header
    +     * @param header to be added
    +     *
    +     */
    +    public void addConfiguredHeader(Header header) {
    +        if (header != null) {
    +            String key = trimToNull(header.getName());
    +            String value = trimToNull(header.getValue());
    +            if (key != null && value != null) {
    +                this.headers.put(key, value);
    +            }
    +        }
    +    }
    +
    +    private String trimToNull(String inputString) {
    --- End diff --
   
    moved this to StringUtils


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user arcadius commented on a diff in the pull request:

    https://github.com/apache/ant/pull/31#discussion_r98559239
 
    --- Diff: src/etc/testcases/taskdefs/get.xml ---
    @@ -98,6 +98,34 @@
         </fail>
       </target>
     
    +  <target name="testTwoHeaders">
    +    <get src="http://www.apache.org/" dest="get.tmp">
    +      <header name="header1" value="header1Value"/>
    +      <header name="header2" value="header2Value"/>
    +    </get>
    +  </target>
    +
    +  <target name="testEmptyHeaders">
    --- End diff --
   
    changed all these names to match the one in the ```GetTest```


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
In reply to this post by apupier
Github user arcadius commented on the issue:

    https://github.com/apache/ant/pull/31
 
    Thank you very much @janmaterne for taking the time to review the code.
    Most appreciated. :+1:
   
    I have made all suggested changes.
   
    Feel free to have a look.
   
    Many thanks.



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
In reply to this post by apupier
Github user arcadius commented on the issue:

    https://github.com/apache/ant/pull/31
 
    Hello @janmaterne ... is there anything i can do to help get this through?
   
    Thank you very much.
   
    Arcadius.
     


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant pull request #31: Feature/bugzilla bug 60628 ant get task to accept arbi...

apupier
In reply to this post by apupier
Github user asfgit closed the pull request at:

    https://github.com/apache/ant/pull/31


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
In reply to this post by apupier
Github user janmaterne commented on the issue:

    https://github.com/apache/ant/pull/31
 
    committed to master branch (Ant 1.10.x).
    thanks for the patch


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] ant issue #31: Feature/bugzilla bug 60628 ant get task to accept arbitrary h...

apupier
In reply to this post by apupier
Github user arcadius commented on the issue:

    https://github.com/apache/ant/pull/31
 
    Thank you very much @janmaterne for your help


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...