Skip to content

Fix binding project LinkWithAttributes generation to prevent unnecess… #1029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rolfbjarne merged 3 commits into from
Oct 31, 2016
Merged

Fix binding project LinkWithAttributes generation to prevent unnecess… #1029

rolfbjarne merged 3 commits into from
Oct 31, 2016

Conversation

@chamons
Copy link
Contributor

@chamons chamons commented Oct 24, 2016

@chamons
Copy link
Contributor Author

chamons commented Oct 24, 2016

Repeat of #1017 but this time not early merged.

@chamons chamons added the do-not-merge Do not merge this pull request label Oct 24, 2016
@chamons
Copy link
Contributor Author

chamons commented Oct 24, 2016

Similar in spirit to #1014 but should fix both XI and XM since logic is shared in this case.

This patch assumes PR 1014 lands first, It might not build w\o the other test changes (I just cherry picked for review).

spouliot
spouliot suggested changes Oct 25, 2016
View reviewed changes
msbuild/Xamarin.MacDev.Tasks.Core/Tasks/PrepareNativeReferencesTaskBase.cs Outdated
}
if (!skipLinkWithGeneration) {
Directory.CreateDirectory (Path.GetDirectoryName (linkWith));
File.WriteAllText (linkWith, text.ToString ());
Copy link
Contributor

@spouliot spouliot Oct 25, 2016

Choose a reason for hiding this comment

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

you might be calling text.ToString() twice (not sure if it's cached, assuming it's a StringBuilder)

Copy link
Contributor Author

@chamons chamons Oct 25, 2016

Choose a reason for hiding this comment

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

@spouliot fixed.

@monojenkins
Copy link
Collaborator

monojenkins commented Oct 26, 2016

Build failure

1 similar comment
@monojenkins
Copy link
Collaborator

monojenkins commented Oct 27, 2016

Build failure

@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 28, 2016

@chamons it doesn't merge cleanly anymore, can you fix?

@chamons
Copy link
Contributor Author

chamons commented Oct 28, 2016

@rolfbjarne Fixed. Will wait for build.

@monojenkins
Copy link
Collaborator

monojenkins commented Oct 28, 2016

Build failure

@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 31, 2016

Test failures are unrelated:

a) introspection-mac/Unified: random crash filed as bug #46102
b) introspection-ios/watchOS: ibtool hung during build - filed as bug #46097

So this is green from jenkins.

@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 31, 2016

This is dependent on PR #1014, which is already merged.

@rolfbjarne rolfbjarne removed the do-not-merge Do not merge this pull request label Oct 31, 2016
@rolfbjarne rolfbjarne merged commit 612f52b into master Oct 31, 2016
@chamons chamons deleted the binding_project_rebuild_fix branch October 31, 2016 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rolfbjarne rolfbjarne rolfbjarne approved these changes

+1 more reviewer

@spouliot spouliot spouliot approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@chamons @monojenkins @rolfbjarne @spouliot @dnfclas