RFR: JMC-6211: Restore Defaults button doesn't revert username and password in Preferences

Elliott Baron ebaron at redhat.com
Tue Mar 5 15:22:47 UTC 2019


Hi Mario,

On 2019-03-05 9:16 a.m., Mario Torre wrote:
> On Mon, 2019-03-04 at 19:29 -0500, Elliott Baron wrote:
>>
>> Marcus has informed me off-list that the above copyright header
>> format
>> is acceptable going forward.
>>
>> I do still need a review for the content of this fix. Would someone
>> be
>> able to take a look?
> 
> Hi Elliott,
> 
> The patch looks good to me, I would like to have the opinion of an
> official reviewer however before you push it.
> 
> A question on the Manifest.
> 
> --- a/application/org.openjdk.jmc.console.ui/META-INF/MANIFEST.MF
> +++ b/application/org.openjdk.jmc.console.ui/META-INF/MANIFEST.MF
> 
> Are those changes in the Manifest really necessary? I take a yes
> because the ui tests are in a different package that need internal
> visibility?
> 

That's correct, the UI tests reside in a different plugin/bundle. 
Exporting these packages with the x-friends directive means that the UI 
tests will have access to the package, but any other code will be warned 
by Eclipse if they try to import the package.

These new imports make some String constants available to the test. We 
could duplicate the constants in the test, but then if they are changed 
in the main code, the test copies would also need to be changed or the 
test will break. There's also one localized String imported by the test, 
which would be harder to work around.

Thanks,
Elliott


More information about the jmc-dev mailing list