Bugzilla – Full Text Bug Listing
| Summary: | Missing baseline warning cannot be disabled | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Olivier Thomann <Olivier_Thomann> | ||||||
| Component: | API Tools | Assignee: | Dani Megert <daniel_megert> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | angvoz.dev, ankur_sharma, curtis.windatt.public, Curtis_Windatt, daniel_megert, markus.kell.r, Michael_Rennie, raksha.vasisht, saurav.sarkar1 | ||||||
| Version: | 3.6 | Flags: | Michael_Rennie:
review+
|
||||||
| Target Milestone: | 3.7 RC1 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Olivier Thomann
This should be fixed for 3.6.1 as this warning should be removed if set to be ignored. Created attachment 170830 [details]
Proposed fix
Michael, please review. this fix brings back the problem described in bug 280619 comment #13 - if a project has settings but a pref does not appear in them we improperly cascade to lookup the workspace setting. Then we need to revisit it. Maybe the previous fix was not exactly what it should be. I think the bigger issue might be that our getSeverity(...) method only really works with preferences that exist in all three scopes (project, instance, default). Maybe we could add the baseline notification pref to the project scope, or have a different method like getSeverity(pref_id, scopes[])? Michael, I let you take care of this since you change the scopes to fix bug 280619. Thanks. The correct fix is to also have that option on the project. Why? Because currently many committers simply ignore the warning and forget to add the baseline and then they introduce API errors in their code without noticing it (see bug 326926 comment 22) for latest example I ran into). The project (owner) itself has no way to force developers to use API Tools since there is no way to have the missing baseline being reported as error. Also, setting the workspace severity to 'Error' would be a bit harsh at this point. I'd really like to see this in 3.7. > Also, setting the workspace severity to 'Error' would be a bit harsh at this
> point.
Actually not. We should consider doing that: if a project decides to enable API Tools then it wants its committers to use it and that can only be signaled by reporting an error for the missing baseline. This also makes sense because some of the API Tools errors have severity 'Error' by default. If a user is annoyed by that then he can easily reduce the severity on the preference page.
Ankur, Curtis, what do you think about setting the severity to 'Error'? I think this would be a great improvement for API Tools. (In reply to comment #10) > Ankur, Curtis, what do you think about setting the severity to 'Error'? I think > this would be a great improvement for API Tools. We've discussed this on our weekly PDE call and decided to go forward with this. Filed bug 331924 to track that. Olivier, I tried your steps and I cannot reproduce the problem using I20101205-2000. Can you still reproduce? If so, can you provide more detailed steps? *** Bug 344417 has been marked as a duplicate of this bug. *** Marking as WORKSFORME. The default setting was changed for 3.7. The problem is real and not related to the default setting. But yes that has exposed it. Steps to repro 1. Have a project with API analysis. 2. Do not add baseline. 3. Enable project specific PDE errors/warning preferences 4. Turn the Missing Baseline preference to Ignore The error marker appears for the project even when the pref is marked Ignore The cause of the error is the pref fetch logic in APIPlugin. When the project specific prefs are enable, the fall back should be Workspace prefs (InstanceScope) and not the Defaults (DefaultScope). The patch has been attached to Bug #344417 Yes I confirm, it is a problem for me too. I downloaded latest build N20110503 and it is still there. I am working on changes to CDT which spans across several releases and by that reason I cannot set definite @since tags. With the change of default settings I always get baseline errors on all CDT projects and not able to reduce those to warnings. The problem with applying the patch Bug #344417 is that if a project has older project specific settings that do not contain a given key (for example the auto-remove filters key from bug 280619) we end up cascading to the workspace preference. What should we be doing in the case a key is not present in the project scope (but expected to be)? Should we initialize it to the workspace value - which is what we do if you set up project specific settings for the first time on a project? If so, then the patch from bug 344417 would fix this, with a minor update to the ApiErrorsAndWarningsConfigurationBlock#addCheckbox(..) method Should a non-existent key in the project scope be initialized to the value from the default scope? Dani / Olivier what does JDT do when a new preference is added as a workspace + project specific setting and the user has not updated existing project specific settings? > Dani / Olivier what does JDT do when a new preference is added as a workspace
> + project specific setting and the user has not updated existing project
> specific settings?
See org.eclipse.jdt.ui.PreferenceConstants.getPreference(String, IJavaProject). In that sense the patch doesn't even go far enough - it should set the context to:
new IScopeContext[] {new ProjectScope(project), InstanceScope.INSTANCE, DefaultScope.INSTANCE};
But I would not make that change now in RC1.
For 3.7 we could consider to keep a list of workspace only preference keys (is there another one besides the missing baseline?) and treat it specially.
Created attachment 194850 [details]
Possible fix for 3.7
Shows direction of a fix using a list of workspace only preference keys. If it turns out that there's only one, then we could inline this directly into ApiPlugin.hasProjectSettings(String, IProject) for 3.7.
Ping! +1 the patch works well. Mike, can you commit this? Dani can't do it any more for RC1. (In reply to comment #22) > Mike, can you commit this? Dani can't do it any more for RC1. done. Verified for RC1 with I20110514-0800. |