[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[maxthomas]

Github user maxthomas commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
see #49 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[Matt Post]

Rebasing changes the history, so I think you can't do that with repos that have 
been pushed, right? In which case merge...

matt (from my phone)

> On Aug 30, 2016, at 3:41 PM, maxthomas  wrote:
> 
> Github user maxthomas commented on the issue:
> 
>https://github.com/apache/incubator-joshua/pull/42
> 
>do you want me to rebase off master, or 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 infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[maxthomas]

Github user maxthomas commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
do you want me to rebase off master, or 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[mjpost]

Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
Yes, please merge master into 7!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[maxthomas]

Github user maxthomas commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
yep, this is ready if you are ready!

i'm happy to merge this into 7. would you like me to do that after this is 
in master? just let me know. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[mjpost]

Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
So am I correct that this is ready to merge into master?

And @maxthomas, you think you can merge this into 7?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[maxthomas]

Github user maxthomas commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
sure, that conflict comment should be fixed. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[KellenSunderland]

Github user KellenSunderland commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
Wow Max, great update.  Many thanks!  I total agree on the questionable 
finalizes.  The code cleanup in general looks great, lots of dead code removed 
there.

As mentioned above there are some conflicts in one commented file, if you 
could fix those it'd be great.  Otherwise looks good to me 👍 .



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[maxthomas]

Github user maxthomas commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
just rebased off latest master


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[mjpost]

Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
So, @maxthomas — what do you think about either of the following:

- You (a) remerge master into this, and then (b) merge master into 7
- Skip (a) and just rework this against the 7 branch?

I think this is good to have in the codebase, but I'm not sure that it's 
critical to have for the 6.1 release. But if it's not too much work to do both, 
I'm not against trying.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[mjpost]

Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
All right; maybe hold off and let me finish and merge in one other 
large-ish commit, and then I'll take a look at this. It's possible it will be 
easier than I anticipate.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...

[mjpost]

Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/42
  
This actually looks really great, but I'm worried about the complexity of 
merging this with the 7, which moves every file and changed a bunch with 
refactoring the feature function interface. How much work went into this, and 
how hard would it be to repeat 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---