Community
Participate
Working Groups
Currently some logging information of ModelAssembler does not provide in which class/ method the warning occurred. That makes it hard for people to place a breakpoint in the the code to find issues.
https://git.eclipse.org/r/24053
In line 282 of ModelAssembler there is System.err.println("Replacing"); //$NON-NLS-1$ causing direct writes to the console. This message should be replaced by a log message and better content too.
Created attachment 242101 [details] ModelAssembler bundle name in log message I also miss the bundle's name the error is related to. Please see my patch which improves the log messages.
WONTFIX, I'm not sure if that changes really helps.
The patch above logs the bundle's name that causes problems when processing the model extensions. This *really* helps in production to find out what bundle exactly causes the issues. I know the patch does not fix all issues mentioned here, but can't we apply the patch to get at least a little bit better logging messages?
Jan, can you converted your patch to a Gerrit review? Why to you replace contributor.getName() with bundle in your patch? Should we not show both?
(In reply to Lars Vogel from comment #6) > Jan, can you converted your patch to a Gerrit review? Yes, will try to do that. I have to setup eclipse gerrit & CLA. (will integrate Markus' suggestion as well) >Why to you replace contributor.getName() with bundle in your patch? - contributor.getName() is the bundle's name. - contributor.getName() is called multiple times (line 101 & 106). - Wherever the bundle's name was used, I replaced it with the variable "bundleName" and added it where it makes sense imo. >Should we not show both? IContributor does only have a name. I guess you thought it's the bundles contributor. But here, I believe, it's the extension contributor (the bundle) Cheers Jan
(In reply to Jan Haensli from comment #7) > (In reply to Lars Vogel from comment #6) > > Jan, can you converted your patch to a Gerrit review? > Yes, will try to do that. I have to setup eclipse gerrit & CLA. > (will integrate Markus' suggestion as well) > > >Why to you replace contributor.getName() with bundle in your patch? > - contributor.getName() is the bundle's name. > - contributor.getName() is called multiple times (line 101 & 106). > - Wherever the bundle's name was used, I replaced it with the variable > "bundleName" and added it where it makes sense imo. > > >Should we not show both? > IContributor does only have a name. I guess you thought it's the bundles > contributor. But here, I believe, it's the extension contributor (the bundle) > > Cheers Jan Feature freeze is today.
(In reply to Jan Haensli from comment #7) > (In reply to Lars Vogel from comment #6) > > Jan, can you converted your patch to a Gerrit review? Due to the time pressure I tried to apply your patch. Unfortunately your patch does not apply and if I copy the modified lines it created syntax errors (bundleName not defined). Please update it onto origin/master.
Thanks Jan, fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=7dbcdcc28c49efb4da854e1de4312b7492bf7240
Patch was for a wrong branch. Sorry. I applied the same changes to current branch: https://git.eclipse.org/r/25576 Please review. Thanks and regards Jan
(In reply to Jan Haensli from comment #11) > Patch was for a wrong branch. Sorry. > > I applied the same changes to current branch: https://git.eclipse.org/r/25576 > Please review. > Thanks and regards > Jan Already did and applied. Thanks a bunch again.
> (In reply to Jan Haensli from comment #11) Jan, can you download the nightly and validate the improved error message? Please leave a comment here.
