Skip to content

Reorganize Xamarin.Android.NamingCustomAttributes. - #139

Merged
jonpryor merged 1 commit into
dotnet:masterfrom
atsushieno:reorganize-attributes
Aug 8, 2016
Merged

Reorganize Xamarin.Android.NamingCustomAttributes.#139
jonpryor merged 1 commit into
dotnet:masterfrom
atsushieno:reorganize-attributes

Conversation

@atsushieno

Copy link
Copy Markdown
Contributor

context: dotnet/java-interop#58
dependent on: atsushieno/java.interop@5583d26

Copypasting the changes on Java.Interop

We have complicated module interdependency between this and xamarin-android,
so this set of changes is to fix the situation.
So far, here is the changeset summary:
- Xamarin.Android.NamingCustomAttributes.shproj is moved to xamarin-android
- there will be new Java.Interop.NamingCustomAttributes.shproj which is for
use in this repository. It will be shrinked in the later commits to have
only required bits in Java/Interop.

Now Xamarin.Android.sln has two NamingCustomAttributes.shproj: one for
imported and then slightly stripped version of the original shared project
in xamarin-android, and renamed and then significantly stripped version
in Java.Interop.

Build adjustment comes next.

@jonpryor

Copy link
Copy Markdown
Contributor

dotnet/java-interop#60 has been merged, so please update the Java.Interop commit hash and finish this work. :-)

@atsushieno
atsushienoforce-pushed the reorganize-attributes branch from 56045ef to 7e26d67CompareAugust 4, 2016 05:27
@atsushieno

Copy link
Copy Markdown
ContributorAuthor

yay, one less dependency!

[AttributeUsage (AttributeTargets.Class,
AllowMultiple=false,
Inherited=false)]
#if !JCW_ONLY_TYPE_NAMES

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need JCW_ONLY_TYPE_NAMES anymore; that was a Java.Interop-ism, so that these types wouldn't be exported from Java.Interop.Tools.JavaCallableWrappers.dll (and other assemblies?).

Since the new Java.Interop partial types are notpublic (i.e. implicitly "internal" unless overridden in another partial class...), JCW_ONLY_TYPE_NAMES shouldn't be needed anywhere anymore.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed those JCW_ONLY_TYPE_NAMES from Android./.cs, while leaving them in Android.Runtime and Java.Interop because at this state it seems quite unwise to make extraneous differences between the corresponding sources in Java.Interop.NamingCustomAttributes and those in this new shared project. Those duplicates could be resolved later, not within this change.

@monojenkins

Copy link
Copy Markdown

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

context: dotnet/java-interop#58
dependent on: atsushieno/java.interop@5583d26
Copypasting the changes on Java.Interop
We have complicated module interdependency between this and xamarin-android,
so this set of changes is to fix the situation.
So far, here is the changeset summary:
- Xamarin.Android.NamingCustomAttributes.shproj is moved to xamarin-android
- there will be new Java.Interop.NamingCustomAttributes.shproj which is for
use in this repository. It will be shrinked in the later commits to have
only required bits in Java/Interop.
Now Xamarin.Android.sln has two NamingCustomAttributes.shproj: one for
imported and then slightly stripped version of the original shared project
in xamarin-android, and renamed and then significantly stripped version
in Java.Interop.
Build adjustment comes next.
@atsushieno
atsushienoforce-pushed the reorganize-attributes branch from 7e26d67 to cd7c541CompareAugust 5, 2016 11:37
#if !JCW_ONLY_TYPE_NAMES
public
#endif // !JCW_ONLY_TYPE_NAMES
partial class ExportAttribute : Attribute {

@jonpryorjonpryorAug 5, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inclusion of this type seems "odd" given that ExportAttribute is also defined Java.Interop/src/Java.Interop.NamingCustomAttributes.

Should this copy be removed? Ditto RegisterAttribute.cs, ExportFieldAttribute.cs, etc.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing those copies break build. Since it should be rather dealt by those who work on those registration types (which I guess is likely you) and I have no idea about the ideal state of them, I don't plan to deal with them. You can make different changes once they are merged.

@jonpryor
jonpryor merged commit ff1c80d into dotnet:masterAug 8, 2016
radekdoulik pushed a commit to radekdoulik/xamarin-android that referenced this pull request Aug 12, 2016
context: dotnet/java-interop#58
dependent on: atsushieno/java.interop@5583d26
Copypasting the changes on Java.Interop
We have complicated module interdependency between this and xamarin-android,
so this set of changes is to fix the situation.
So far, here is the changeset summary:
- Xamarin.Android.NamingCustomAttributes.shproj is moved to xamarin-android
- there will be new Java.Interop.NamingCustomAttributes.shproj which is for
use in this repository. It will be shrinked in the later commits to have
only required bits in Java/Interop.
Now Xamarin.Android.sln has two NamingCustomAttributes.shproj: one for
imported and then slightly stripped version of the original shared project
in xamarin-android, and renamed and then significantly stripped version
in Java.Interop.
Build adjustment comes next.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Feb 6, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@atsushieno@jonpryor@monojenkins@dnfclas