[GitHub] ant-ivy pull request #43: Generics and other fixes in tests and tutorials

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

[GitHub] ant-ivy pull request #43: Generics and other fixes in tests and tutorials

twogee
GitHub user twogee opened a pull request:

    https://github.com/apache/ant-ivy/pull/43

    Generics and other fixes in tests and tutorials

    foreach loops are more terse than iterators, and they go well together with generic -- here's the first stab 😃

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

    $ git pull https://github.com/twogee/ant-ivy master

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

    https://github.com/apache/ant-ivy/pull/43.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 #43
   
----
commit 8b9f2d5177c849b37809ab38035e1d757d963314
Author: twogee <[hidden email]>
Date:   2017-06-10T06:56:44Z

    Generics and other fixes in tests and tutorials

----


---
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-ivy issue #43: Generics and other fixes in tests and tutorials

twogee
Github user janmaterne commented on the issue:

    https://github.com/apache/ant-ivy/pull/43
 
    Java7 supports the diamond operator and Ivy's baseline is now Java7. So you could use that (e.g. CCFilter.java).
   
    HMFilter: one advantage of foreach is having a meaningful iterator name. So renaming s/string/value/ would be more readable.
   
    HelloIvy: maybe you could change the hello-message: s/hello ivy !/Hello Ivy!/
   
    src/example/multi-project/projects/find/src/find/Main.java: maybe also change from raw Collection to Collection<File>, so the foreach could also be typed. This would impact FindFile.
    --> better in a separate PR
   
    Same for src/example/multi-project/projects/list/src/list/Main.java
    --> better in a separate PR
   
    Same src/example/multi-project/projects/size/src/size/FileSize.java
    --> better in a separate PR
   
    test/java/org/apache/ivy/core/module/id/ModuleIdTest.java
    You changed the semantics:
      assertFalse(moduleId.equals(null)) --> asserts that moduleId is not null (otherwise a NPE would be thrown) and it's equal-comparison returns false
      assertNotNull(moduleId) --> 'just' assert that moduleId is not null
    --> maybe just place the notnull before the assertfalse?



---
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-ivy issue #43: Generics and other fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    Thank you for comments, I'll split the PR and correct according to your suggestions. Should I remove common-colections altogether and use collections with generics from Java runtime?


---
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-ivy pull request #43: Generics and other fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43


---
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-ivy pull request #43: Generics and other fixes in tests and tutorials

twogee
In reply to this post by twogee
GitHub user twogee reopened a pull request:

    https://github.com/apache/ant-ivy/pull/43

    Generics and other fixes in tests and tutorials

    foreach loops are more terse than iterators, and they go well together with generic -- here's the first stab 😃

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

    $ git pull https://github.com/twogee/ant-ivy master

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

    https://github.com/apache/ant-ivy/pull/43.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 #43
   
----
commit 8b9f2d5177c849b37809ab38035e1d757d963314
Author: twogee <[hidden email]>
Date:   2017-06-10T06:56:44Z

    Generics and other fixes in tests and tutorials

commit 8f35a1d28e795833e6a095ff55ecc37f3db882aa
Author: twogee <[hidden email]>
Date:   2017-06-10T22:26:54Z

    More foreach loops

commit 314dfa80d4e815ba3c689cd3ad0d508209665bf6
Author: twogee <[hidden email]>
Date:   2017-06-11T02:38:47Z

    More generics…

----


---
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-ivy issue #43: Generics and other fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    I wont split this PR. Adress these in this PR:
    - diamond operator
    - renaming
    - ModuleIdTest
   
    The other things should be done in a new PR (PRs shouldnt be too big ;)
    I wouldnt remove commons-collections as it is an example how to use external libs with Ivy.


---
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-ivy issue #43: Generics and other fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    Here comes a bunch of new changes: I had to go for Commons Collections 4 to use generics, and threw in Commons CLI 1.4 for a good measure. I left HTTP Client 2.0 for the future lest this PR grows further 😃


---
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-ivy issue #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    Sorry for getting carried away with diamonds 😃... I put back
    `assertFalse(moduleId.equals(null));`
    but that assertion is equivalent to
    `assertFalse(null instanceOf ModuleId)`
    which is always true, isn't it?


---
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-ivy issue #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    Nop ... yes. You're right with 'null instanceof SomeClass'==true.
    But the two assertions are different:
    moduleId = null;
    assertFalse(moduleId.equals(null);; --> this will throw a NullPointerException


---
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-ivy issue #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    That's exactly why I introduced assertNotNull :-)


---
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-ivy issue #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    I mean, why use an NPE when one can have an assertion that fails? It's a cleaner approach.


---
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-ivy pull request #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43


---
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-ivy issue #43: Generics and related fixes in tests and tutorials

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

    https://github.com/apache/ant-ivy/pull/43
 
    Merged that, thanks.
    Hint for future PRs: smaller are better (easier to review)


---
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...