Skip to content

Conversation

@jpobst
Copy link
Contributor

@jpobst jpobst commented May 17, 2022

Context: dotnet/android#6643
Context: dotnet/java-interop#727

Use <AndroidNamespaceReplacement> instead of Metadata to specify namespaces. This allows us to significantly cut down the amount of work needed to ensure proper namespaces.

image

For example, all of the following lines are needed simply to correctly capitalize AndroidX:

<attr path="/api/package[@name='androidx.core.app']" name="managedName">AndroidX.Core.App</attr>
<attr path="/api/package[@name='androidx.core.content']" name="managedName">AndroidX.Core.Content</attr>
<attr path="/api/package[@name='androidx.core.database']" name="managedName">AndroidX.Core.Database</attr>
<attr path="/api/package[@name='androidx.core.graphics']" name="managedName">AndroidX.Core.Graphics</attr>
<attr path="/api/package[@name='androidx.core.graphics.drawable']" name="managedName">AndroidX.Core.Graphics.Drawable</attr>
<attr path="/api/package[@name='androidx.core.hardware.display']" name="managedName">AndroidX.Core.Hardware.Display</attr>
<attr path="/api/package[@name='androidx.core.hardware.fingerprint']" name="managedName">AndroidX.Core.Hardware.Fingerprint</attr>
<attr path="/api/package[@name='androidx.core.internal']" name="managedName">AndroidX.Core.Internal</attr>
<attr path="/api/package[@name='androidx.core.internal.view']" name="managedName">AndroidX.Core.Internal.View</attr>

This can be replaced with:

<AndroidNamespaceReplacement Include='Androidx' Replacement='AndroidX' />

Additionally add "standard" replacements for combined words that require special capitalization:

<AndroidNamespaceReplacement Include='accessibilityservice' Replacement='AccessibilityService' />

This ensures that these words are always capitalized consistently across all the packages in this repo.

Going forward, we will rarely need to manually handle new namespaces. We should only need to worry if a new combined-word namespace is added.

(This is why #558 is important, since we will not need to actively adjust every new namespace.)

@jpobst jpobst marked this pull request as ready for review May 24, 2022 18:31
@jpobst jpobst requested a review from moljac May 24, 2022 18:31
@moljac
Copy link
Contributor

moljac commented May 25, 2022

Going forward, we will rarely need to manually handle new namespaces. We should only need to worry if a new combined-word namespace is added.

How are we going to detect those rare cases? I mean, exceptions in the rules are usually the most difficult to detect and fix.

My presumption was:

if Androidx.* was detected => new namespace appeared => check the whole namespace for the correctness.

I don't remember case when it failed.

The reason there is Metadata.Namespaces.xml was simply attempt to separate metadata generation for bindings and it was the result of collecting packagenames with MSBuild Custom Task Nuget that could be added (if desired) to bindings project:

My attempt to learn MSBuild when I had more time:

https://github.com/HolisticWare-Xamarin-Tools/HolisticWare.Xamarin.Tools.Bindings.XamarinAndroid.MetadataXmlSpitter/blob/master/source/HolisticWare.Xamarin.Tools.Bindings.XamarinAndroid.ApiXmlSpitter/PackagesToNamespacesSpitter.cs

@jpobst
Copy link
Contributor Author

jpobst commented May 25, 2022

How are we going to detect those rare cases? I mean, exceptions in the rules are usually the most difficult to detect and fix.

Every new namespace will have to be added to the published-namespaces.txt file in order to pass CI. It is now no longer possible for a new namespace to get added (or removed) to a package without us knowing.

It should be much easier to review changes to that file in a PR diff rather than having to examine every api-diff for new namespaces.

Detecting these rare cases if we generally do not have to make adjustments for namespaces was absolutely a concern. It is the reason published-namespaces.txt was added. 😁

@jpobst jpobst merged commit 0aaa542 into main May 26, 2022
@jpobst jpobst deleted the ns-replace branch May 26, 2022 13:28
@jpobst jpobst mentioned this pull request May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants