[GitHub] ant-ivyde pull request #7: Fix IVYDE-386

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde pull request #7: Fix IVYDE-386

bodewig
GitHub user jonl-percsolutions-com opened a pull request:

    https://github.com/apache/ant-ivyde/pull/7

    Fix IVYDE-386

    According to Eclipse documentation, IContributionItem can, in no way, be relied on to be a MenuManager and in Eclipse Oxygen, this code is causing an exception as documented in IVYDE-386, by myself.  Adding this check will resolve the Exception, which is causing menu building to fail for not only Ivy, but for other plugins as well.
   
    The circumstances around why this is occurring are unknown to myself.
   
    https://help.eclipse.org/mars/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fui%2Fmenus%2FCommandContributionItem.html
    https://issues.apache.org/jira/browse/IVYDE-386

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

    $ git pull https://github.com/jonl-percsolutions-com/ant-ivyde patch-1

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

    https://github.com/apache/ant-ivyde/pull/7.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 #7
   
----
commit 9ccc319311e01c202193516880cec5a94d853409
Author: J L <jonl-percsolutions-com@...>
Date:   2018-02-28T17:35:14Z

    Fix IVYDE-386
   
    According to Eclipse documentation, IContributionItem can, in no way, be relied on to be a MenuManager and in Eclipse Oxygen, this code is causing an exception as documented in IVYDE-386, by myself.  Adding this check will resolve the Exception, which is causing menu building to fail for not only Ivy, but for other plugins as well.
   
    The circumstances around why this is occurring are unknown to myself.
   
    https://help.eclipse.org/mars/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fui%2Fmenus%2FCommandContributionItem.html
    https://issues.apache.org/jira/browse/IVYDE-386

----


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

bodewig
Github user twogee commented on the issue:

    https://github.com/apache/ant-ivyde/pull/7
 
    There is a [regression](https://github.com/apache/ant-ivy/commit/ae27582d1ba0cb6c2b18d30a81cb1b82033069c3) in URLHandler to be dealt with
   
    ```
    [pde-build]     [javac] schemaStream = URLHandlerRegistry.getDefault().openStream(schema, null);
    [pde-build]     [javac]                                               ^^^^^^^^^^
    [pde-build]     [javac] The method openStream(URL) in the type URLHandler is not applicable for the arguments (URL, null)
    ```



---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

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

    https://github.com/apache/ant-ivyde/pull/7
 
    Could you please rebase on the latest master?


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

bodewig
In reply to this post by bodewig
Github user jonl-percsolutions-com commented on the issue:

    https://github.com/apache/ant-ivyde/pull/7
 
    I believe I did it correctly and it is now rebased.


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

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

    https://github.com/apache/ant-ivyde/pull/7
 
    >> There is a regression in URLHandler to be dealt with
   
    Actually, that's not a regression. It's a fix to a regression that I introduced in one of the snapshots. 2.4.x had `openStream` which accepted just the URL[1] and in one of the snapshots, while working on introducing the timeout feature, I introduced a newer API which broke backward compatibility. Maarten pointed me to this issue in one the dev mailing list and I then reverted that commit to switch back to the older state that is compatible with 2.4.x.
   
    So the commit that you did in ant-ivyde repo, a few hours ago is the correct one. Thanks for fixing that.
   
    [1] https://github.com/apache/ant-ivy/blob/2.4.x/src/java/org/apache/ivy/util/url/URLHandler.java#L166


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

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

    https://github.com/apache/ant-ivyde/pull/7
 
    Sorry about a loose remark and for pursuing the deprecations [overagressively](https://github.com/apache/ant-ivyde/commit/d2983ea950fc93daa2db8bb6ef17958450fd34e3).
    What I would appreciate is force push after rebase that would trigger a new Jenkins build. I don't know if there is a better way of doing that.


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

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

    https://github.com/apache/ant-ivyde/pull/7
 
    @jonl-percsolutions-com Thank you for reporting the issue and then raising this PR. I looked at the changes in this PR and it looks like this change will silently ignore items that aren't of type `MenuManager`. I think a better way to deal with this issue is something like this https://github.com/apache/ant-ivyde/pull/8. Would it be possible for you to try that change?
   
   
   



---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde issue #7: Fix IVYDE-386

bodewig
In reply to this post by bodewig
Github user jonl-percsolutions-com commented on the issue:

    https://github.com/apache/ant-ivyde/pull/7
 
    @jaikiran Your fix is much better.  I did not understand enough about the code to see whether or not it was intentional for those to be MenuManager objects or not.  I have updated to the beta build with your pull request and can confirm that it fixes the issue I was seeing an all menus in eclipse are being built correctly.


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivyde pull request #7: Fix IVYDE-386

bodewig
In reply to this post by bodewig
Github user jonl-percsolutions-com closed the pull request at:

    https://github.com/apache/ant-ivyde/pull/7


---

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