You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by twogee <gi...@git.apache.org> on 2017/06/10 19:58:50 UTC
[GitHub] ant-ivy pull request #43: Generics and other fixes in tests and tutorials
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 <g....@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and other fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy pull request #43: Generics and other fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 <g....@gmail.com>
Date: 2017-06-10T06:56:44Z
Generics and other fixes in tests and tutorials
commit 8f35a1d28e795833e6a095ff55ecc37f3db882aa
Author: twogee <g....@gmail.com>
Date: 2017-06-10T22:26:54Z
More foreach loops
commit 314dfa80d4e815ba3c689cd3ad0d508209665bf6
Author: twogee <g....@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and other fixes in tests and tutorials
Posted by janmaterne <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy pull request #43: Generics and related fixes in tests and tutorials
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and related fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and related fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy pull request #43: Generics and other fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and other fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and other fixes in tests and tutorials
Posted by janmaterne <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and related fixes in tests and tutorials
Posted by twogee <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and related fixes in tests and tutorials
Posted by janmaterne <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] ant-ivy issue #43: Generics and related fixes in tests and tutorials
Posted by janmaterne <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org