Skip to content

Conversation

@spouliot
Copy link
Contributor

No description provided.

@spouliot spouliot added this to the d15-7 milestone Mar 16, 2018
@spouliot spouliot added the do-not-merge Do not merge this pull request label Mar 16, 2018
@spouliot
Copy link
Contributor Author

Work in progress (there's more docs to sync).
There's a lot of whitespace changes but it's either here or every time it's sync back...

@bradumbaugh please double check - it's largely your changes over our own

@chamons we need to fix 2100 for mmp :(

@bradumbaugh
Copy link

bradumbaugh commented Mar 16, 2018

@spouliot Looks good. One change request: please put internal anchors such as <a name="MT0087"/> as the only element on a line, surrounded by newlines. Also, please put a space before the closing slash.

This:

<a name="MT0055" />

### MT0055: The Xcode path '*' does not exist. 

Instead of:

### <a name="MT0055"/>MT0055: The Xcode path '*' does not exist. 

@chamons
Copy link
Contributor

chamons commented Mar 16, 2018

Blah. At least 2100 is a simple error code. Patch in a second.

@bradumbaugh
Copy link

@spouliot Made an update above - I'd mistakenly left off a space after the # character.

@chamons
Copy link
Contributor

chamons commented Mar 16, 2018

@spouliot https://git.io/vxmHH should work for 2100

@spouliot
Copy link
Contributor Author

@chamons please PR that on master but without mtouch-errors.md changes, since it will be overwritten when merging back (from 15.7)

@chamons
Copy link
Contributor

chamons commented Mar 16, 2018

On it.

@spouliot
Copy link
Contributor Author

@bradumbaugh not sure I follow, there's no change for MT0055, i.e. https://github.com/xamarin/xamarin-macios/pull/3771/files#diff-656045af32acf6dda51d8e5dd079d5e2L238

or are you suggest a global change ?

The first commit is only a merge back from the version in xamarin-docs-pr which I assume was good enough, format wise :) We can discuss the future format since I don't want to keep the current one either inside #3570

IOW this PR is only so we don't overwrite your changes (in xamarin-docs-pr) when we're ready to release/publish 15.7 publicly

@bradumbaugh
Copy link

bradumbaugh commented Mar 16, 2018

@spouliot Yes, I'm suggesting a global change for that doc. The current doc is going through a localization process that requires the anchors to be placed as I've described above, so that's a change that's in-process.

[edit] @spouliot If you'd like to wait until we have the change in the docs repository before making it here, that's fine - but we'll still need to circle back. Apologies for the confusion.

@chamons
Copy link
Contributor

chamons commented Mar 16, 2018

#3773 for the 2100 fixes

@spouliot
Copy link
Contributor Author

@bradumbaugh whats the timeline for the localization change ?

@VincentDondain
Copy link
Contributor

@spouliot about #3768 (comment), you didn't fix MM0132 here, only the mtouch error.

Copy link
Contributor

@VincentDondain VincentDondain left a comment

Choose a reason for hiding this comment

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

LGTM except we should also fix MM0132 🙏

@bradumbaugh
Copy link

@spouliot I will make the necessary change and reply back today.

@bradumbaugh
Copy link

@spouliot The changes to anchors we discussed last week are in now in the master in the docs repo.

@monojenkins
Copy link
Collaborator

Build failure

@spouliot spouliot added the pr-change-not-shipping The PR only touch files that are not shipped to customers label Mar 23, 2018
@monojenkins
Copy link
Collaborator

Build success
Build comment file:

Provisioning succeeded
Build succeeded
API Diff (from stable)
API Diff (from PR only)
Generator Diff
Test run succeeded


@spouliot spouliot removed the do-not-merge Do not merge this pull request label Apr 10, 2018
@spouliot
Copy link
Contributor Author

@dalexsoto please review, in particular the binding documents (to make sure I did not miss anything you added).

@bradumbaugh one, hopefully, final review please :)

@monojenkins
Copy link
Collaborator

Build success
Build comment file:

Provisioning succeeded
Build succeeded
API Diff (from stable)
API Diff (from PR only)
Generator Diff
Test run succeeded


@monojenkins
Copy link
Collaborator

Build failure
Build comment file:

Provisioning succeeded
Build succeeded
API Diff (from stable)
API Diff (from PR only)
Generator Diff
🔥 Test run failed 🔥

Test results

4 tests failed, 75 tests passed.

Failed tests

  • monotouch-test/iOS Unified 32-bits - simulator/Debug: TimedOut
  • introspection/iOS Unified 32-bits - simulator/Debug: TimedOut
  • monotouch-test/iOS Unified 32-bits - simulator/Debug (static registrar): TimedOut
  • monotouch-test/iOS Unified 32-bits - simulator/Release (all optimizations): TimedOut

@spouliot
Copy link
Contributor Author

Failures are unrelated, something went wrong with 32bits sim/contact authorization -> https://github.com/xamarin/maccore/issues/737

Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link

@bradumbaugh bradumbaugh left a comment

Choose a reason for hiding this comment

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

@spouliot Went through these changes, compared them to master, did a test publish to make sure things rendered correctly. A few changes requested - all pretty minor.

### DesignatedDefaultCtorAttribute

When this attribute is applied to the interface definition it will generate
a `[DesignatedInitializer]` attribute on the default (generated) constructor

Choose a reason for hiding this comment

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

add a comma after "default (generated) constructor"

```

When the `WrapAttribute` is used inside a `Category` you need to include `This` as
When the `[WrapAttribute]` is used inside a `Category` you need to include `This` as

Choose a reason for hiding this comment

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

  • Should "Category" be "[Category]" here?
  • Should "This" be lowercase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it should be (in order of preference) [Category] or CategoryAttribute or [CategoryAttribute]
This is correct in this wrapped context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I re-worded the paragraph to explain why. Please review it again :)

Choose a reason for hiding this comment

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

Looks good.


When the `WrapAttribute` is used inside a `Category` you need to include `This` as
When the `[WrapAttribute]` is used inside a `Category` you need to include `This` as
the first argument inside the `Wrap` signature, for example:

Choose a reason for hiding this comment

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

change to: "signature. For example:"

## RequiresSuperAttribute
### RequiresSuperAttribute

This is a specialized subclass of the `[Advice]` attribute can be used

Choose a reason for hiding this comment

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

that can be used

### RequiresSuperAttribute

This is a specialized subclass of the `[Advice]` attribute can be used
to hint the developer that overriding a method **requires** a call to

Choose a reason for hiding this comment

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

to the developer

title: Xamarin.iOS Analysis Rules
dateupdated: 2018-02-15
title: "Xamarin.iOS analysis rules"
ms.topic: article

Choose a reason for hiding this comment

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

change to:

ms.topic: troubleshooting

- **Fix:** Double click on your iOS project, go to Build > iOS Build and for Release|iPhone, check the LLVM optimizing compiler option.
## XIA0007: UseLLVMRule

- **Problem:** For Release|iPhone configuration we recommend enabling the LLVM compiler which generates code that is faster to execute at the expense of build time.

Choose a reason for hiding this comment

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

  • **Release|iPhone** (bold)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nicer but that text is extracted (by a tool) from Xamarin.iOS.Analysis.targets and it's shown in UI in places where markdown does not apply. IOW it cannot be applied to it's origin and will come back unformatted each time.

## XIA0007: UseLLVMRule

- **Problem:** For Release|iPhone configuration we recommend enabling the LLVM compiler which generates code that is faster to execute at the expense of build time.
- **Fix:** Double click on your iOS project, go to Build > iOS Build and for Release|iPhone, check the LLVM optimizing compiler option.

Choose a reason for hiding this comment

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

  • **Build > iOS Build** (bold)
  • **Release|iPhone**

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

all: xamarin-analysis-doc
cp mtouch-errors.md $(WEBSITE_DOCS)/guides/ios/troubleshooting/mtouch-errors/index.md
cp mtouch-errors.md $(WEBSITE_DOCS)/docs/ios/troubleshooting/mtouch-errors.md
cp binding_objc_libs.md $(WEBSITE_DOCS)/guides/cross-platform/macios/binding/objective-c-libraries/index.md

Choose a reason for hiding this comment

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

This document lives at:

docs/cross-platform/macios/binding/objective-c-libraries.md

cp xamarin-ios-analysis.md $(WEBSITE_DOCS)/guides/ios/troubleshooting/xamarin-ios-analysis/index.md
cp binding_types_reference_guide.md $(WEBSITE_DOCS)/docs/cross-platform/macios/binding/binding-types-reference.md
cp xamarin-ios-analysis.md $(WEBSITE_DOCS)/ios/troubleshooting/xamarin-ios-analysis.md
cp optimizations.md $(WEBSITE_DOCS)/guides/cross-platform/macios/build-optimizations/index.md

Choose a reason for hiding this comment

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

I don't see this doc in the existing docs.microsoft.com TOC...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

optimizations.md is new in 15.7. IIRC we discussed it with you a few week (months now?) back. c.c. @rolfbjarne

Copy link
Member

Choose a reason for hiding this comment

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

I created a PR for the documentation repo, which was postponed until the move to docs.microsoft.com: https://github.com/xamarin/documentation/pull/2845

Choose a reason for hiding this comment

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

Right - apologies for missing this. I'll look at adding this content today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since it's new in 15.7 so it can wait for the release (since it won't be usable content until then)

Choose a reason for hiding this comment

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

Sounds good, I'll just get it ready.

Choose a reason for hiding this comment

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

PR here for build optimizations (waiting for 15.7 to merge): https://github.com/MicrosoftDocs/xamarin-docs-pr/pull/204

@bradumbaugh
Copy link

@spouliot One further question - @VincentDondain requested that we also publish https://github.com/xamarin/xamarin-macios/blob/master/docs/website/generator-errors.md. It will require similar changes to the anchors, etc. Should we include that as part of this PR or wait until it is published before doing so?

@spouliot
Copy link
Contributor Author

@bradumbaugh for generator-errors.md we'll wait for 15.8 since we'll want IDE support (double click to error) and beside being too late for 15.7 we will do it "one page per error" from day one :)

Copy link

@bradumbaugh bradumbaugh left a comment

Choose a reason for hiding this comment

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

Need to update the Makefile for the location of the build optimizations doc (hasn't been merged yet, but the Makefile is inconsistent with its future location)

cp xamarin-ios-analysis.md $(WEBSITE_DOCS)/guides/ios/troubleshooting/xamarin-ios-analysis/index.md
cp binding_types_reference_guide.md $(WEBSITE_DOCS)/docs/cross-platform/macios/binding/binding-types-reference.md
cp xamarin-ios-analysis.md $(WEBSITE_DOCS)/ios/troubleshooting/xamarin-ios-analysis.md
cp optimizations.md $(WEBSITE_DOCS)/guides/cross-platform/macios/build-optimizations/index.md

Choose a reason for hiding this comment

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

Staged the build optimizations content in a PR here: https://github.com/MicrosoftDocs/xamarin-docs-pr/pull/204.

Build optimizations will live here:

docs/cross-platform/macios/optimizations.md

-diff -u $(WEBSITE_DOCS)/docs/cross-platform/macios/binding/objective-c-libraries.md binding_objc_libs.md
-diff -u $(WEBSITE_DOCS)/docs/cross-platform/macios/binding/binding-types-reference.md binding_types_reference_guide.md
-diff -u $(WEBSITE_DOCS)/docs/ios/troubleshooting/xamarin-ios-analysis.md xamarin-ios-analysis.md
-diff -u $(WEBSITE_DOCS)/docs/cross-platform/macios/build-optimizations/index.md optimizations.md

Choose a reason for hiding this comment

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

Same as above; build optimizations doc will live here:

docs/cross-platform/macios/optimizations.md

@monojenkins
Copy link
Collaborator

Build failure
Build comment file:

Provisioning succeeded
Build succeeded
API Diff (from stable)
API Diff (from PR only)
Generator Diff


@monojenkins
Copy link
Collaborator

Build success
Build comment file:

Provisioning succeeded
Build succeeded
API Diff (from stable)
API Diff (from PR only)
Generator Diff
Test run succeeded


@spouliot spouliot merged commit c80edce into dotnet:d15-7 Apr 17, 2018
@spouliot spouliot deleted the d15-7-docsync branch April 17, 2018 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-change-not-shipping The PR only touch files that are not shipped to customers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants