[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

[chtyim]

Github user chtyim commented on the issue:

https://github.com/apache/twill/pull/65
  
Changes LGTM.


---

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

[cbaenziger]

Github user cbaenziger commented on the issue:

https://github.com/apache/twill/pull/65
  
Hi @chtyim, thank you for the review. Could you please expand on what you 
see concerning in the `pom.xml`? For reference, I need to provide a 2.6 target 
as it is the first version to provide the `ContainerId.fromString()` API.


---

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

[chtyim]

Github user chtyim commented on the issue:

https://github.com/apache/twill/pull/65
  
The change looks mostly ok to me, except for the change in the pom.xml 
profile.


---

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

[cbaenziger]

Github user cbaenziger commented on the issue:

https://github.com/apache/twill/pull/65
  
Thank you for the super fast code review @serranom! I think all comments 
have been addressed and the code should be complete now. Please let me know if 
you see anything else necessary. It is great working with Twill! Hopefully I 
can provide some other ideas as I work with it.


---

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

[cbaenziger]

Github user cbaenziger commented on the issue:

https://github.com/apache/twill/pull/65
  
I added a few omissions and corrected the parent  `pom.xml` from my 
original submission. Now the new code is actually being used and I have updated 
the default target to Hadoop 2.6 and removed the generic `hadoop.version` as 
suggested.


---